Changeset 123904 in webkit

Timestamp:

Jul 27, 2012 12:25:43 PM (12 years ago)

Author:

mitz@apple.com

Message:

Hit testing near a column break can return a result from an adjacent column when there is leading
​https://bugs.webkit.org/show_bug.cgi?id=92524

Reviewed by Anders Carlsson.

Source/WebCore:

The fix for <​http://webkit.org/b/92311> relied on the existence of a pagination strut for
detecting that a line was at the beginning of a new column. However, when a line naturally
falls at the beginning of a column, there is no pagination strut, and the check failed.

Test: fast/multicol/hit-test-end-of-column-with-line-height.html

• rendering/InlineFlowBox.h:

(WebCore::InlineFlowBox::InlineFlowBox): Added initializer for new member variable.
(InlineFlowBox): Added m_isFirstAfterPageBreak member variable.

• rendering/RenderBlock.cpp:

(WebCore::RenderBlock::positionForPointWithInlineChildren): Changed the test for whether a
line was at the beginning of a column from relying on a pagination strut to checking
isFirstAfterPageBreak(). Also refined the hit test itself to include the leading above such
a line.
(WebCore::RenderBlock::adjustLinePositionForPagination): Added calls to
setIsFirstAfterPageBreak() to first reset this flag, then set it to true if necessary.

• rendering/RootInlineBox.h:

(WebCore::RootInlineBox::isFirstAfterPageBreak): Added this accessor.
(WebCore::RootInlineBox::setIsFirstAfterPageBreak): Ditto.

LayoutTests:

• fast/multicol/hit-test-end-of-column-with-line-height-expected.txt: Added.
• fast/multicol/hit-test-end-of-column-with-line-height.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/multicol/hit-test-end-of-column-with-line-height-expected.txt (added)
• LayoutTests/fast/multicol/hit-test-end-of-column-with-line-height.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/InlineFlowBox.h (modified) (2 diffs)
• Source/WebCore/rendering/RenderBlock.cpp (modified) (3 diffs)
• Source/WebCore/rendering/RootInlineBox.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-07-27  Dan Bernstein  <mitz@apple.com>
+
+        Hit testing near a column break can return a result from an adjacent column when there is leading
+        https://bugs.webkit.org/show_bug.cgi?id=92524
+
+        Reviewed by Anders Carlsson.
+
+        * fast/multicol/hit-test-end-of-column-with-line-height-expected.txt: Added.
+        * fast/multicol/hit-test-end-of-column-with-line-height.html: Added.
+
 2012-07-27  Mike West  <mkwst@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-07-27  Dan Bernstein  <mitz@apple.com>
+
+        Hit testing near a column break can return a result from an adjacent column when there is leading
+        https://bugs.webkit.org/show_bug.cgi?id=92524
+
+        Reviewed by Anders Carlsson.
+
+        The fix for <http://webkit.org/b/92311> relied on the existence of a pagination strut for
+        detecting that a line was at the beginning of a new column. However, when a line naturally
+        falls at the beginning of a column, there is no pagination strut, and the check failed.
+
+        Test: fast/multicol/hit-test-end-of-column-with-line-height.html
+
+        * rendering/InlineFlowBox.h:
+        (WebCore::InlineFlowBox::InlineFlowBox): Added initializer for new member variable.
+        (InlineFlowBox): Added m_isFirstAfterPageBreak member variable.
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::positionForPointWithInlineChildren): Changed the test for whether a
+        line was at the beginning of a column from relying on a pagination strut to checking
+        isFirstAfterPageBreak(). Also refined the hit test itself to include the leading above such
+        a line.
+        (WebCore::RenderBlock::adjustLinePositionForPagination): Added calls to
+        setIsFirstAfterPageBreak() to first reset this flag, then set it to true if necessary.
+        * rendering/RootInlineBox.h:
+        (WebCore::RootInlineBox::isFirstAfterPageBreak): Added this accessor.
+        (WebCore::RootInlineBox::setIsFirstAfterPageBreak): Ditto.
+
 2012-07-27  John J. Barton  <johnjbarton@johnjbarton.com>
 

trunk/Source/WebCore/rendering/InlineFlowBox.h

@@ -50 +50 @@
         , m_hasAnnotationsBefore(false)
         , m_hasAnnotationsAfter(false)
+        , m_isFirstAfterPageBreak(false)
 #ifndef NDEBUG
         , m_hasBadChildList(false)
@@ -322 +323 @@
     unsigned m_hasAnnotationsBefore : 1;
     unsigned m_hasAnnotationsAfter : 1;
+    bool m_isFirstAfterPageBreak : 1;
 
     unsigned m_lineBreakBidiStatusEor : 5; // WTF::Unicode::Direction

trunk/Source/WebCore/rendering/RenderBlock.cpp

@@ -4927 +4927 @@
             firstRootBoxWithChildren = root;
 
-        if (root->paginationStrut() && pointInLogicalContents.y() < root->logicalTop())
+        if (root->isFirstAfterPageBreak() && pointInLogicalContents.y() < root->lineTopWithLeading())
             break;
 
@@ -6962 +6962 @@
     logicalOffset += delta;
     lineBox->setPaginationStrut(0);
+    lineBox->setIsFirstAfterPageBreak(false);
     LayoutUnit pageLogicalHeight = pageLogicalHeightForOffset(logicalOffset);
     bool hasUniformPageLogicalHeight = !inRenderFlowThread() || enclosingRenderFlowThread()->regionsHaveUniformLogicalHeight();
@@ -6985 +6986 @@
             delta += remainingLogicalHeight;
             lineBox->setPaginationStrut(remainingLogicalHeight);
-        }
-    }
+            lineBox->setIsFirstAfterPageBreak(true);
+        }
+    } else if (remainingLogicalHeight == pageLogicalHeight && lineBox != firstRootBox())
+        lineBox->setIsFirstAfterPageBreak(true);
 }
 

trunk/Source/WebCore/rendering/RootInlineBox.h

@@ -56 +56 @@
     LayoutUnit paginationStrut() const { return m_paginationStrut; }
     void setPaginationStrut(LayoutUnit s) { m_paginationStrut = s; }
+
+    bool isFirstAfterPageBreak() const { return m_isFirstAfterPageBreak; }
+    void setIsFirstAfterPageBreak(bool isFirstAfterPageBreak) { m_isFirstAfterPageBreak = isFirstAfterPageBreak; }
 
     LayoutUnit paginatedLineWidth() const { return m_paginatedLineWidth; }