Bug 1340127 - Consider different bidi control/override values when deciding whether to consider a frame first or last. r=jfkthame
authorL. David Baron <dbaron@dbaron.org>
Wed, 01 Mar 2017 10:04:27 -0800
changeset 374368 eb6b998cfa79ea7eddbba71ccb3e9c8e327691aa
parent 374367 af1edb14a70783428bd19329869bd06e6ae74b44
child 374369 1a34bdb0c31a69914719d5b68e05781e4e62b81c
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjfkthame
bugs1340127
milestone54.0a1
Bug 1340127 - Consider different bidi control/override values when deciding whether to consider a frame first or last. r=jfkthame I believe the reordering of the first/last check across the code that delves into letter frames is an improvement, but a currently undectable one, since it appears that we don't currently allow ::first-letter pseudo-elements to break across lines, even in the presence of multi-character ::first-letters that are broken by 'word-break:break-all'.
layout/base/nsBidiPresUtils.cpp
--- a/layout/base/nsBidiPresUtils.cpp
+++ b/layout/base/nsBidiPresUtils.cpp
@@ -1016,31 +1016,43 @@ nsBidiPresUtils::TraverseFrames(nsBlockI
      * It's important to get the next sibling and next continuation *before*
      * handling the frame: If we encounter a forced paragraph break and call
      * ResolveParagraph within this loop, doing GetNextSibling and
      * GetNextContinuation after that could return a bidi continuation that had
      * just been split from the original childFrame and we would process it
      * twice.
      */
     nsIFrame* nextSibling = childFrame->GetNextSibling();
-    bool isLastFrame = !childFrame->GetNextContinuation();
-    bool isFirstFrame = !childFrame->GetPrevContinuation();
 
     // If the real frame for a placeholder is a first letter frame, we need to
     // drill down into it and include its contents in Bidi resolution.
     // If not, we just use the placeholder.
     nsIFrame* frame = childFrame;
     if (nsGkAtoms::placeholderFrame == childFrame->GetType()) {
       nsIFrame* realFrame =
         nsPlaceholderFrame::GetRealFrameForPlaceholder(childFrame);
       if (realFrame->GetType() == nsGkAtoms::letterFrame) {
         frame = realFrame;
       }
     }
 
+    auto DifferentBidiValues = [](nsIFrame* aFrame1, nsIFrame* aFrame2) {
+      nsStyleContext* sc1 = aFrame1->StyleContext();
+      nsStyleContext* sc2 = aFrame2->StyleContext();
+      return GetBidiControl(sc1) != GetBidiControl(sc2) ||
+             GetBidiOverride(sc1) != GetBidiOverride(sc2);
+    };
+
+    nsIFrame* nextContinuation = frame->GetNextContinuation();
+    nsIFrame* prevContinuation = frame->GetPrevContinuation();
+    bool isLastFrame = !nextContinuation ||
+                       DifferentBidiValues(frame, nextContinuation);
+    bool isFirstFrame = !prevContinuation ||
+                        DifferentBidiValues(frame, prevContinuation);
+
     char16_t controlChar = 0;
     char16_t overrideChar = 0;
     if (frame->IsFrameOfType(nsIFrame::eBidiInlineContainer)) {
       if (!(frame->GetStateBits() & NS_FRAME_FIRST_REFLOW)) {
         nsContainerFrame* c = static_cast<nsContainerFrame*>(frame);
         MOZ_ASSERT(c == do_QueryFrame(frame),
                    "eBidiInlineContainer must be a nsContainerFrame subclass");
         c->DrainSelfOverflowList();