WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
[patch]
Patch
bug-88376-20120910144501.patch (text/plain), 18.70 KB, created by
Allan Sandfeld Jensen
on 2012-09-10 05:45:36 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Allan Sandfeld Jensen
Created:
2012-09-10 05:45:36 PDT
Size:
18.70 KB
patch
obsolete
>Subversion Revision: 128046 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index e196e26aa3d4068576e9df93a7764ac768205b2c..7cf2b784c859ccc7958e31f4eceb976a5ab0feb2 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,25 @@ >+2012-08-13 Allan Sandfeld Jensen <allan.jensen@nokia.com> >+ >+ Incorrect rect-based hit-test result when hit-test region includes culled inlines >+ https://bugs.webkit.org/show_bug.cgi?id=88376 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Move the handling of culled inlines from HitTestResult::addNodeToRectBasedTestResult to >+ InlineFlowBox::nodeAtPoint. This makes it possible to fix a number of bugs with how >+ culled inlines were handled. They are now checked after all their children, and may >+ terminate area-based hit-testing if they contain the whole area. >+ >+ Test: fast/dom/nodesFromRect/nodesFromRect-culled-inline.html >+ >+ * rendering/HitTestResult.cpp: >+ (WebCore::HitTestResult::addNodeToRectBasedTestResult): >+ * rendering/InlineFlowBox.cpp: >+ (WebCore::InlineFlowBox::nodeAtPoint): >+ (WebCore::InlineFlowBox::culledNodeAtPoint): >+ * rendering/InlineFlowBox.h: >+ (InlineFlowBox): >+ > 2012-09-10 Alexander Pavlov <apavlov@chromium.org> > > Web Inspector: [Elements] Poor performance upon continuous attribute changes >diff --git a/Source/WebCore/rendering/HitTestResult.cpp b/Source/WebCore/rendering/HitTestResult.cpp >index 8063c812ea16efcfc1a5ea4bea1174fc028a83c1..d20cf68e227291f698e0c645010e3ebc02fa9048 100644 >--- a/Source/WebCore/rendering/HitTestResult.cpp >+++ b/Source/WebCore/rendering/HitTestResult.cpp >@@ -685,21 +685,6 @@ bool HitTestResult::addNodeToRectBasedTestResult(Node* node, const HitTestReques > mutableRectBasedTestResult().add(node); > > bool regionFilled = rect.contains(locationInContainer.boundingBox()); >- // FIXME: This code (incorrectly) attempts to correct for culled inline nodes. See https://bugs.webkit.org/show_bug.cgi?id=85849. >- if (node->renderer()->isInline() && !regionFilled) { >- for (RenderObject* curr = node->renderer()->parent(); curr; curr = curr->parent()) { >- if (!curr->isRenderInline()) >- break; >- >- // We need to make sure the nodes for culled inlines get included. >- RenderInline* currInline = toRenderInline(curr); >- if (currInline->alwaysCreateLineBoxes()) >- break; >- >- if (currInline->visibleToHitTesting() && currInline->node()) >- mutableRectBasedTestResult().add(currInline->node()->shadowAncestorNode()); >- } >- } > return !regionFilled; > } > >@@ -720,21 +705,6 @@ bool HitTestResult::addNodeToRectBasedTestResult(Node* node, const HitTestReques > mutableRectBasedTestResult().add(node); > > bool regionFilled = rect.contains(locationInContainer.boundingBox()); >- // FIXME: This code (incorrectly) attempts to correct for culled inline nodes. See https://bugs.webkit.org/show_bug.cgi?id=85849. >- if (node->renderer()->isInline() && !regionFilled) { >- for (RenderObject* curr = node->renderer()->parent(); curr; curr = curr->parent()) { >- if (!curr->isRenderInline()) >- break; >- >- // We need to make sure the nodes for culled inlines get included. >- RenderInline* currInline = toRenderInline(curr); >- if (currInline->alwaysCreateLineBoxes()) >- break; >- >- if (currInline->visibleToHitTesting() && currInline->node()) >- mutableRectBasedTestResult().add(currInline->node()->shadowAncestorNode()); >- } >- } > return !regionFilled; > } > >diff --git a/Source/WebCore/rendering/InlineFlowBox.cpp b/Source/WebCore/rendering/InlineFlowBox.cpp >index 9ad0fcfa12c833b0a1d3c774c9f87543f75d4c18..c61ae1c8f03adba84ef07b4e480e1a66e0e4b98b 100644 >--- a/Source/WebCore/rendering/InlineFlowBox.cpp >+++ b/Source/WebCore/rendering/InlineFlowBox.cpp >@@ -981,11 +981,44 @@ bool InlineFlowBox::nodeAtPoint(const HitTestRequest& request, HitTestResult& re > return false; > > // Check children first. >+ // We need to account for culled inline parents of the hit-tested nodes, so that they may also get included in area-based hit-tests. >+ RenderObject* culledParent = 0; > for (InlineBox* curr = lastChild(); curr; curr = curr->prevOnLine()) { >- if ((curr->renderer()->isText() || !curr->boxModelObject()->hasSelfPaintingLayer()) && curr->nodeAtPoint(request, result, locationInContainer, accumulatedOffset, lineTop, lineBottom)) { >+ if (curr->renderer()->isText() || !curr->boxModelObject()->hasSelfPaintingLayer()) { >+ RenderObject* newParent = 0; >+ // Culled parents are only relevant for area-based hit-tests, so ignore it in point-based ones. >+ if (locationInContainer.isRectBasedTest()) { >+ newParent = curr->renderer()->parent(); >+ if (newParent == renderer()) >+ newParent = 0; >+ } >+ // Check the culled parent after all its children have been checked, to do this we wait until >+ // we are about to test an element with a different parent. >+ if (newParent != culledParent) { >+ if (!newParent || !newParent->isDescendantOf(culledParent)) { >+ while (culledParent && culledParent != renderer() && culledParent != newParent) { >+ if (culledNodeAtPoint(culledParent, request, result, locationInContainer, accumulatedOffset)) { >+ renderer()->updateHitTestResult(result, locationInContainer.point() - toLayoutSize(accumulatedOffset)); >+ return true; >+ } >+ culledParent = culledParent->parent(); >+ } >+ } >+ culledParent = newParent; >+ } >+ if (curr->nodeAtPoint(request, result, locationInContainer, accumulatedOffset, lineTop, lineBottom)) { >+ renderer()->updateHitTestResult(result, locationInContainer.point() - toLayoutSize(accumulatedOffset)); >+ return true; >+ } >+ } >+ } >+ // Check the last culled ancestors if any. >+ while (culledParent && culledParent != renderer()) { >+ if (culledNodeAtPoint(culledParent, request, result, locationInContainer, accumulatedOffset)) { > renderer()->updateHitTestResult(result, locationInContainer.point() - toLayoutSize(accumulatedOffset)); > return true; > } >+ culledParent = culledParent->parent(); > } > > // Now check ourselves. Pixel snap hit testing. >@@ -1020,6 +1053,21 @@ bool InlineFlowBox::nodeAtPoint(const HitTestRequest& request, HitTestResult& re > return false; > } > >+bool InlineFlowBox::culledNodeAtPoint(const RenderObject* renderer, const HitTestRequest& request, HitTestResult& result, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset) >+{ >+ if (!renderer->isRenderInline() || !renderer->visibleToHitTesting()) >+ return false; >+ >+ const RenderInline* renderInline = toRenderInline(renderer); >+ ASSERT(!renderInline->alwaysCreateLineBoxes()); >+ FloatRect adjustedRect = renderInline->linesBoundingBox(); >+ adjustedRect.moveBy(accumulatedOffset); >+ if (!locationInContainer.intersects(adjustedRect)) >+ return false; >+ >+ return !result.addNodeToRectBasedTestResult(renderer->node(), request, locationInContainer, adjustedRect); >+} >+ > void InlineFlowBox::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset, LayoutUnit lineTop, LayoutUnit lineBottom) > { > LayoutRect overflowRect(visualOverflowRect(lineTop, lineBottom)); >diff --git a/Source/WebCore/rendering/InlineFlowBox.h b/Source/WebCore/rendering/InlineFlowBox.h >index 5a34f6c6579ba2a2c131f51b4b9d08fe04b75f57..ead0c6163d5a7faa3d3419e3d6852032c11da726 100644 >--- a/Source/WebCore/rendering/InlineFlowBox.h >+++ b/Source/WebCore/rendering/InlineFlowBox.h >@@ -297,6 +297,7 @@ private: > void addTextBoxVisualOverflow(InlineTextBox*, GlyphOverflowAndFallbackFontsMap&, LayoutRect& logicalVisualOverflow); > void addReplacedChildOverflow(const InlineBox*, LayoutRect& logicalLayoutOverflow, LayoutRect& logicalVisualOverflow); > void constrainToLineTopAndBottomIfNeeded(LayoutRect&) const; >+ bool culledNodeAtPoint(const RenderObject*, const HitTestRequest&, HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset); > > protected: > OwnPtr<RenderOverflow> m_overflow; >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index a800751d1002d8e1e785cdd48ef099f208f3e9cb..0163eb46930f769d5ab9e5fa43986bc67ebf27b8 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,22 @@ >+2012-08-13 Allan Sandfeld Jensen <allan.jensen@nokia.com> >+ >+ Incorrect rect-based hit-test result when hit-test region includes culled inlines >+ https://bugs.webkit.org/show_bug.cgi?id=88376 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Renames the existing nodesFromRect-culled-inlines.html test to nodesFromRect-inline-image.html, >+ because it did not test any culled inlines anymore, and replace it with a new test that does >+ test culled inlines. >+ >+ * fast/dom/nodesFromRect/nodesFromRect-culled-inlines-expected.txt: >+ * fast/dom/nodesFromRect/nodesFromRect-culled-inlines.html: >+ * fast/dom/nodesFromRect/nodesFromRect-inline-image-expected.txt: Added. >+ * fast/dom/nodesFromRect/nodesFromRect-inline-image.html: Added. >+ * fast/dom/nodesFromRect/resources/nodesFromRect.js: >+ (checkRect): A new check method that gives more information on failures. >+ (nodesFromRectAsString): Returns a formated string representing the nodes returned by nodesFromRect. >+ > 2012-09-10 Allan Sandfeld Jensen <allan.jensen@nokia.com> > > Unreviewed gardening, unskip tests passing after r128040 landed. >diff --git a/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-culled-inlines-expected.txt b/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-culled-inlines-expected.txt >index d7454dbee36f122914db8cbe2f9f5f4f7925bfc5..ebed24791b417f85d4e508bcc784b0d68ce08828 100644 >--- a/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-culled-inlines-expected.txt >+++ b/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-culled-inlines-expected.txt >@@ -1,4 +1,10 @@ >- PASS All correct nodes found for rect >+Document::nodesFromRect : culled inlines - bug 88376 >+ >+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". >+ >+ >+PASS All correct nodes found for rect >+PASS All correct nodes found for rect > PASS All correct nodes found for rect > PASS All correct nodes found for rect > PASS All correct nodes found for rect >diff --git a/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-culled-inlines.html b/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-culled-inlines.html >index b0d905806ca11b78cd01191eefcf77876842549a..b2e06524c1b5c91f8f74cb0d368b6f20580c4053 100644 >--- a/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-culled-inlines.html >+++ b/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-culled-inlines.html >@@ -1,48 +1,52 @@ > <!DOCTYPE html> > <html> > <head> >- <title>Document::nodesFromRect test - bug 85849</title> >- <script src="../../js/resources/js-test-pre.js"></script> >- <script src="resources/nodesFromRect.js"></script> >- <script type="application/javascript"> >- function runTest() >- { >- var e = {}; >- >- // Set up shortcut access to elements >- e['html'] = document.getElementsByTagName("html")[0]; >- ['body', 'span', 'img'].forEach(function(a) { >- e[a] = document.getElementById(a); >- }); >- >- window.scrollTo(0, 0); >- >- /* Point based test over the img only. */ >- check(20, 20, 0, 0, 0, 0, [e.img]); >- /* Rect based test over the img only. */ >- check(20, 20, 5, 5, 5, 5, [e.img]); >- >- /* Note that for the tests below, the img bounds are considered to be (99, 99). */ >- /* Point based test over the img and the span. */ >- check(0, 99, 0, 0, 0, 0, [e.img]); >- /* Rect based test over the img and the span with the img fully covering the hit region. */ >- check(0, 98, 0, 1, 1, 0, [e.img]); >- /* Rect based test over the img and the span with the img not fully covering the hit region. */ >- /* FIXME: This fails due to: https://bugs.webkit.org/show_bug.cgi?id=88376 */ >- check(0, 98, 0, 1, 2, 0, [e.img, e.span]); >- /* Rect based test over the entire img. */ >- check(0, 0, 0, 99, 99, 0, [e.img]); >- } >- </script> >+ <title>Document::nodesFromRect : culled inlines - bug 88376</title> >+ <script src="../../js/resources/js-test-pre.js"></script> >+ <script src="resources/nodesFromRect.js"></script> >+ <style> >+ #sandbox { >+ position: absolute; >+ left: 0px; >+ top: 0px; >+ width: 400px; >+ height: 200px; >+ } >+ #sandbox p { font: 16px Ahem; } >+ </style> > </head> >-<body id="body" style="padding: 0; margin: 0;"> >- <span id="span" style="margin: 0; padding: 0; font-size:36px"> >- <img id="img" width="100" height="100" style="background-color: black; margin: 0; padding: 0;" /> >- </span> >+<body id="body"> >+ <div id=sandbox> >+ <p><span id=culledinline><span id=wordinline1>word1</span> <span id=wordinline2>word2</span></span> <span id=wordinline3>word3</span></p> >+ </div> >+ >+ <p id="description"></p> >+ <span id="console"></span> >+ <script type="application/javascript"> >+ function runTest() >+ { >+ description(document.title); >+ window.scrollTo(0, 0); >+ /* Rect based test over word1 only. */ >+ checkRect(30, 19, 8, 8, "'word1'"); >+ /* Rect based test over the word2 only. */ >+ checkRect(126, 19, 8, 8, "'word2'"); >+ /* Rect based test over the word3 only. */ >+ checkRect(222, 19, 8, 8, "'word3'"); >+ /* Rect based test between word1 and word2. */ >+ checkRect(84, 19, 8, 8, "' '"); >+ /* Rect based test over and outside word1. */ >+ checkRect(70, 19, 20, 8, "' ', 'word1', SPAN#wordinline1, SPAN#culledinline"); >+ /* Rect based test over word1 and word2. */ >+ checkRect(70, 19, 40, 8, "'word2', SPAN#wordinline2, ' ', 'word1', SPAN#wordinline1, SPAN#culledinline"); >+ /* Rect based test over word2 and word3. */ >+ checkRect(170, 19, 40, 8, "'word3', SPAN#wordinline3, ' ', 'word2', SPAN#wordinline2, SPAN#culledinline, P"); > >- <span id="console" style="position: absolute; top: 150px;"></span> >- <script> runTest();</script> >- <script src="../../js/resources/js-test-post.js"></script> >+ document.getElementById('sandbox').style.display = 'none'; >+ } >+ runTest(); >+ </script> >+ <script src="../../js/resources/js-test-post.js"></script> > </body> > </html> > >diff --git a/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-inline-image-expected.txt b/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-inline-image-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..64c67df6f4fa628670dcd3f55145da9cb370fb4e >--- /dev/null >+++ b/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-inline-image-expected.txt >@@ -0,0 +1,21 @@ >+ >+Document::nodesFromRect : inline image - bug 85849 >+ >+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". >+ >+ >+PASS All correct nodes found for rect >+PASS All correct nodes found for rect >+PASS All correct nodes found for rect >+PASS All correct nodes found for rect >+PASS All correct nodes found for rect >+PASS All correct nodes found for rect >+PASS All correct nodes found for rect >+PASS All correct nodes found for rect >+PASS All correct nodes found for rect >+PASS All correct nodes found for rect >+PASS All correct nodes found for rect >+PASS successfullyParsed is true >+ >+TEST COMPLETE >+ >diff --git a/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-inline-image.html b/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-inline-image.html >new file mode 100644 >index 0000000000000000000000000000000000000000..aacdc9c6917e3fd36dcb5740a538729ce52b1acc >--- /dev/null >+++ b/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-inline-image.html >@@ -0,0 +1,67 @@ >+<!DOCTYPE html> >+<html> >+<head> >+ <title>Document::nodesFromRect : inline image - bug 85849</title> >+ <script src="../../js/resources/js-test-pre.js"></script> >+ <script src="resources/nodesFromRect.js"></script> >+ <style> >+ #sandbox { >+ position: absolute; >+ left: 0px; >+ top: 0px; >+ width: 400px; >+ height: 200px; >+ } >+ #sandbox #container { padding: 2px; padding-bottom: 10px; } >+ #sandbox #container span { font-size: 36px; } >+ #sandbox img { background-color: black; } >+ </style> >+</head> >+<body id="body"> >+ <div id=sandbox> >+ <div id=container> >+ <span> >+ <img width="100" height="100"></img> >+ </span> >+ </div> >+ </div> >+ >+ <p id="description"></p> >+ <span id="console"></span> >+ <script type="application/javascript"> >+ function runTest() >+ { >+ description(document.title); >+ window.scrollTo(0, 0); >+ /* Point based test over the img only. */ >+ checkRect(20, 20, 1, 1, "IMG"); >+ /* Rect based test over the img only. */ >+ checkRect(15, 15, 10, 10, "IMG"); >+ /* Rect based test over the div only. */ >+ checkRect(0, 0, 2, 2, "DIV#container"); >+ /* Rect based test over the span only. */ >+ checkRect(3, 103, 2, 2, "SPAN"); >+ >+ /* Note that for the tests below, the img bounds are considered to be (2, 2) x (100, 100). */ >+ /* Rect based test over the entire img. */ >+ checkRect(2, 2, 100, 100, "IMG"); >+ /* Point based test over the img and the span. */ >+ checkRect(2, 99, 1, 1, "IMG"); >+ /* Rect based test over the img and the span with the img fully covering the hit region. */ >+ checkRect(2, 98, 2, 2, "IMG"); >+ /* Rect based test over the img and the span with the img not fully covering the hit region. */ >+ checkRect(3, 101, 2, 5, "IMG, SPAN"); >+ /* Rect based test over the img, span and their container. */ >+ checkRect(3, 101, 2, 18, "IMG, SPAN, DIV#container"); >+ /* Rect based test over just span and its container. */ >+ checkRect(3, 103, 2, 16, "SPAN, DIV#container"); >+ /* Rect based test over the img that is not over span with the img not fully covering the hit region. */ >+ checkRect(1, 1, 3, 3, "IMG, DIV#container"); >+ >+ } >+ runTest(); >+ </script> >+ <script src="../../js/resources/js-test-post.js"></script> >+</body> >+</html> >+
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=163101">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 88376
:
155301
|
155480
|
155517
|
155529
|
155550
|
157978
|
163101
|
163103
|
163875
|
164391