Bug 1308876 - Don't continue reflow after deciding we need to try aga…

…in due to page-break-inside:avoid. r=mats The primary patch in this bug causes fewer dirty reflows, which leads to lines being out-of-date for the reason described in the comment. This causes incorrect layout of some references sections on wikipedia, for which a simplified testcase is included. This bug was not caught by anything in our test suite, but I noticed it while browsing wikipedia (since I use a build that has my patches in it for my regular browsing). MozReview-Commit-ID: 4hTQpGS2pZH
mozilla · Jul 13, 2017 · e094153 · e094153
1 parent 8487157
commit e094153
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 0 deletions.
diff --git a/layout/generic/nsBlockFrame.cpp b/layout/generic/nsBlockFrame.cpp
@@ -4716,6 +4716,7 @@ nsBlockFrame::PlaceLine(BlockReflowInput& aState,
     aState.mReflowStatus.SetInlineLineBreakBeforeAndReset();
     // Reflow the line again when we reflow at our new position.
     aLine->MarkDirty();
+    *aKeepReflowGoing = false;
     return true;
   }
 
@@ -4727,6 +4728,7 @@ nsBlockFrame::PlaceLine(BlockReflowInput& aState,
     if (ShouldAvoidBreakInside(aState.mReflowInput)) {
       // All our content doesn't fit, start on the next page.
       aState.mReflowStatus.SetInlineLineBreakBeforeAndReset();
+      *aKeepReflowGoing = false;
     } else {
       // Push aLine and all of its children and anything else that
       // follows to our next-in-flow.
@@ -4735,6 +4737,10 @@ nsBlockFrame::PlaceLine(BlockReflowInput& aState,
     return true;
   }
 
+  // Note that any early return before this update of aState.mBCoord
+  // must either (a) return false or (b) set aKeepReflowGoing to false.
+  // Otherwise we'll keep reflowing later lines at an incorrect
+  // position, and we might not come back and clean up the damage later.
   aState.mBCoord = newBCoord;
 
   // Add the already placed current-line floats to the line

diff --git a/layout/reftests/columns/break-avoid-line-position-1-ref.html b/layout/reftests/columns/break-avoid-line-position-1-ref.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html lang="en" dir="ltr">
+<meta charset="UTF-8"/>
+<title>Wikipedia References Multicol Bug</title>
+
+<style>
+  html { overflow: hidden }
+  div { width: 15em; margin-right: 2em; float: left; }
+  p { margin: 0 }
+</style>
+
+<div>
+  <p>abcdefghijkl0000000000 abcdefghijkl1111111111 abcdefghijkl2222222222</p>
+  <p>abcdefghijkl0000000000 abcdefghijkl1111111111 abcdefghijkl2222222222 abcdefghijkl3333333333</p>
+  <p>abcdefghijkl0000000000 abcdefghijkl1111111111 abcdefghijkl2222222222</p>
+</div>
+<div>
+  <p>abcdefghijkl0000000000 abcdefghijkl1111111111 abcdefghijkl2222222222 abcdefghijkl3333333333</p>
+  <p>abcdefghijkl0000000000 abcdefghijkl1111111111 abcdefghijkl2222222222</p>
+  <p>abcdefghijkl0000000000 abcdefghijkl1111111111</p>
+</div>
diff --git a/layout/reftests/columns/break-avoid-line-position-1.html b/layout/reftests/columns/break-avoid-line-position-1.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html lang="en" dir="ltr">
+<meta charset="UTF-8"/>
+<title>Wikipedia References Multicol Bug</title>
+
+<style>
+  html { overflow: hidden }
+  div { width: 32em; column-count: 2; column-gap: 2em; }
+  p { page-break-inside:avoid; margin: 0; }
+</style>
+
+<div>
+  <p>abcdefghijkl0000000000 abcdefghijkl1111111111 abcdefghijkl2222222222</p>
+  <p>abcdefghijkl0000000000 abcdefghijkl1111111111 abcdefghijkl2222222222 abcdefghijkl3333333333</p>
+  <p>abcdefghijkl0000000000 abcdefghijkl1111111111 abcdefghijkl2222222222</p>
+  <p>abcdefghijkl0000000000 abcdefghijkl1111111111 abcdefghijkl2222222222 abcdefghijkl3333333333</p>
+  <p>abcdefghijkl0000000000 abcdefghijkl1111111111 abcdefghijkl2222222222</p>
+  <p>abcdefghijkl0000000000 abcdefghijkl1111111111</p>
+</div>
diff --git a/layout/reftests/columns/reftest.list b/layout/reftests/columns/reftest.list
@@ -38,3 +38,4 @@ fuzzy-if(browserIsRemote&&winWidget,140,276) == fieldset-columns-001.html fields
 == dynamic-change-with-overflow-1.html dynamic-change-with-overflow-1-ref.html
 == dynamic-text-indent-1.html dynamic-text-indent-1-ref.html
 == dynamic-text-indent-2.html dynamic-text-indent-2-ref.html
+== break-avoid-line-position-1.html break-avoid-line-position-1-ref.html