Bug 1340127 - Consider different bidi control/override values when deciding whether to consider a frame first or last. r=jfkthame, a=gchang
authorL. David Baron <dbaron@dbaron.org>
Wed, 01 Mar 2017 10:04:27 -0800
changeset 376555 0da8d305ce0072bf900c6d17d5372b907ec72c34
parent 376554 7363ef4a20e5d3ee808aa3f1eaa15254d01a16c2
child 376556 49c606518e60a1f51c3793d929b3c8670b9be63a
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjfkthame, gchang
bugs1340127
milestone53.0a2
Bug 1340127 - Consider different bidi control/override values when deciding whether to consider a frame first or last. r=jfkthame, a=gchang 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();