- Revision
- 141252
- Author
- [email protected]
- Date
- 2013-01-30 04:13:34 -0800 (Wed, 30 Jan 2013)
Log Message
[Chromium] Fix find in page rects for overflowing content.
https://bugs.webkit.org/show_bug.cgi?id=104924
Reviewed by Julien Chaffraix.
If a div has overflowing content, we should only normalise its
coordinates against the renderview or the containing scrollable block.
TEST=WebFrameTest.FindInPageMatchRects
* src/FindInPageCoordinates.cpp:
(WebKit::enclosingScrollableAncestor):
Helper function to find the enclosing containing block with an overflow clip.
(WebKit::toNormalizedRect):
Pass in the container as an argument.
(WebKit::findInPageRectFromAbsoluteRect):
Compute the container for toNormalizedRect using enclosingScrollableAncestor.
* tests/WebFrameTest.cpp:
Add expectations for new tests in WebFrameTest::FindInPageMatchRects and WebFrameTest::FindInPage.
* tests/data/find.html:
Add test case for <select> element.
* tests/data/find_in_page_frame.html:
Add test cases:
- Result 15, 16 tests that containing <div> with style float:left and
height:0px does not impact coordinate normalization.
- Result 17, 18 tests that matches with absolute positioning are normalized containing
relative positioned block, even if there is a closer parent block with overflow clip.
- Result 19 adds a test case for <select> element.
Modified Paths
Diff
Modified: trunk/Source/WebKit/chromium/ChangeLog (141251 => 141252)
--- trunk/Source/WebKit/chromium/ChangeLog 2013-01-30 12:07:19 UTC (rev 141251)
+++ trunk/Source/WebKit/chromium/ChangeLog 2013-01-30 12:13:34 UTC (rev 141252)
@@ -1,3 +1,34 @@
+2013-01-30 John Knottenbelt <[email protected]>
+
+ [Chromium] Fix find in page rects for overflowing content.
+ https://bugs.webkit.org/show_bug.cgi?id=104924
+
+ Reviewed by Julien Chaffraix.
+
+ If a div has overflowing content, we should only normalise its
+ coordinates against the renderview or the containing scrollable block.
+
+ TEST=WebFrameTest.FindInPageMatchRects
+
+ * src/FindInPageCoordinates.cpp:
+ (WebKit::enclosingScrollableAncestor):
+ Helper function to find the enclosing containing block with an overflow clip.
+ (WebKit::toNormalizedRect):
+ Pass in the container as an argument.
+ (WebKit::findInPageRectFromAbsoluteRect):
+ Compute the container for toNormalizedRect using enclosingScrollableAncestor.
+ * tests/WebFrameTest.cpp:
+ Add expectations for new tests in WebFrameTest::FindInPageMatchRects and WebFrameTest::FindInPage.
+ * tests/data/find.html:
+ Add test case for <select> element.
+ * tests/data/find_in_page_frame.html:
+ Add test cases:
+ - Result 15, 16 tests that containing <div> with style float:left and
+ height:0px does not impact coordinate normalization.
+ - Result 17, 18 tests that matches with absolute positioning are normalized containing
+ relative positioned block, even if there is a closer parent block with overflow clip.
+ - Result 19 adds a test case for <select> element.
+
2013-01-30 Dominik Röttsches <[email protected]>
[HarfBuzz] Remove the HarfBuzz-old code
Modified: trunk/Source/WebKit/chromium/src/FindInPageCoordinates.cpp (141251 => 141252)
--- trunk/Source/WebKit/chromium/src/FindInPageCoordinates.cpp 2013-01-30 12:07:19 UTC (rev 141251)
+++ trunk/Source/WebKit/chromium/src/FindInPageCoordinates.cpp 2013-01-30 12:13:34 UTC (rev 141252)
@@ -49,11 +49,21 @@
namespace WebKit {
-static FloatRect toNormalizedRect(const FloatRect& absoluteRect, const RenderObject* renderer)
+static const RenderBlock* enclosingScrollableAncestor(const RenderObject* renderer)
{
+ ASSERT(!renderer->isRenderView());
+
+ // Trace up the containingBlocks until we reach either the render view or a scrollable object.
+ const RenderBlock* container = renderer->containingBlock();
+ while (!container->hasOverflowClip() && !container->isRenderView())
+ container = container->containingBlock();
+ return container;
+}
+
+static FloatRect toNormalizedRect(const FloatRect& absoluteRect, const RenderObject* renderer, const RenderBlock* container)
+{
ASSERT(renderer);
- const RenderBlock* container = renderer->containingBlock();
ASSERT(container || renderer->isRenderView());
if (!container)
return FloatRect();
@@ -92,26 +102,27 @@
return FloatRect();
// Normalize the input rect to its container block.
- FloatRect normalizedRect = toNormalizedRect(inputRect, baseRenderer);
+ const RenderBlock* baseContainer = enclosingScrollableAncestor(baseRenderer);
+ FloatRect normalizedRect = toNormalizedRect(inputRect, baseRenderer, baseContainer);
// Go up across frames.
- for (const RenderObject* renderer = baseRenderer->containingBlock(); renderer; ) {
+ for (const RenderBox* renderer = baseContainer; renderer; ) {
// Go up the render tree until we reach the root of the current frame (the RenderView).
- for (const RenderBlock* container = renderer->containingBlock(); container;
- renderer = container, container = container->containingBlock()) {
+ while (!renderer->isRenderView()) {
+ const RenderBlock* container = enclosingScrollableAncestor(renderer);
// Compose the normalized rects.
- FloatRect normalizedBoxRect = toNormalizedRect(renderer->absoluteBoundingBoxRect(), renderer);
+ FloatRect normalizedBoxRect = toNormalizedRect(renderer->absoluteBoundingBoxRect(), renderer, container);
normalizedRect.scale(normalizedBoxRect.width(), normalizedBoxRect.height());
normalizedRect.moveBy(normalizedBoxRect.location());
- if (normalizedRect.isEmpty())
- return normalizedRect;
+ renderer = container;
}
+ ASSERT(renderer->isRenderView());
+
// Jump to the renderer owning the frame, if any.
- ASSERT(renderer->isRenderView());
renderer = renderer->frame() ? renderer->frame()->ownerRenderer() : 0;
}
Modified: trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp (141251 => 141252)
--- trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp 2013-01-30 12:07:19 UTC (rev 141251)
+++ trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp 2013-01-30 12:13:34 UTC (rev 141252)
@@ -1349,6 +1349,14 @@
// "bar4" is surrounded by <span>, but the focusable node should be the parent <div>.
EXPECT_EQ(WebString::fromUTF8("DIV"), frame->document().focusedNode().nodeName());
+ // Find in <select> content.
+ EXPECT_FALSE(frame->find(findIdentifier, WebString::fromUTF8("bar5"), options, false, 0));
+ // If there are any matches, stopFinding will set the selection on the found text.
+ // However, we do not expect any matches, so check that the selection is null.
+ frame->stopFinding(false);
+ range = frame->selectionRange();
+ ASSERT_TRUE(range.isNull());
+
webView->close();
}
@@ -1517,9 +1525,10 @@
webView->layout();
webkit_support::RunAllPendingMessages();
+ // Note that the 'result 19' in the <select> element is not expected to produce a match.
static const char* kFindString = "result";
static const int kFindIdentifier = 12345;
- static const int kNumResults = 16;
+ static const int kNumResults = 19;
WebFindOptions options;
WebString searchText = WebString::fromUTF8(kFindString);
@@ -1610,6 +1619,13 @@
EXPECT_TRUE(webMatchRects[13].y < webMatchRects[12].y);
EXPECT_TRUE(webMatchRects[12].y < webMatchRects[14].y);
+ // Result 16 should be below result 15.
+ EXPECT_TRUE(webMatchRects[15].y > webMatchRects[14].y);
+
+ // Result 18 should be normalized with respect to the position:relative div, and not it's
+ // immediate containing div. Consequently, result 18 should be above result 17.
+ EXPECT_TRUE(webMatchRects[17].y > webMatchRects[18].y);
+
// Resizing should update the rects version.
webView->resize(WebSize(800, 600));
webkit_support::RunAllPendingMessages();
Modified: trunk/Source/WebKit/chromium/tests/data/find.html (141251 => 141252)
--- trunk/Source/WebKit/chromium/tests/data/find.html 2013-01-30 12:07:19 UTC (rev 141251)
+++ trunk/Source/WebKit/chromium/tests/data/find.html 2013-01-30 12:13:34 UTC (rev 141252)
@@ -3,4 +3,7 @@
<input value="foo2 bar2 baz2">
<textarea>foo3 bar3 baz3</textarea>
<div contentEditable="true">foo4 <span>bar4</span> baz4</div>
+<select>
+<option value="foo5">bar5</option>
+</select>
</body>
Modified: trunk/Source/WebKit/chromium/tests/data/find_in_page_frame.html (141251 => 141252)
--- trunk/Source/WebKit/chromium/tests/data/find_in_page_frame.html 2013-01-30 12:07:19 UTC (rev 141251)
+++ trunk/Source/WebKit/chromium/tests/data/find_in_page_frame.html 2013-01-30 12:13:34 UTC (rev 141252)
@@ -25,17 +25,34 @@
height: 100px;
position: fixed;
}
+div.relative-overflow {
+ position: relative;
+ overflow: hidden;
+ height: 100px;
+}
+div.margin-overflow {
+ overflow: hidden;
+ margin-top:20px;
+ height: 50px;
+}
+div.absolute {
+ position: absolute;
+ top: 0px;
+ height: 20px;
+ width: 80px;
+ background-color: "green";
+}
</style>
</head>
<body>
This is a test.
-</br></br>
-</br></br>
-</br></br>
+<br>
+<br>
+<br>
Foo bar.
-</br></br>
-</br></br>
-</br></br>
+<br>
+<br>
+<br>
result 02
<div class="transform">
result 03
@@ -48,13 +65,13 @@
<div class="scroll">
result 07
Foo bar.
-</br></br>
-</br></br>
-</br></br>
+<br>
+<br>
+<br>
result 08
</div>
result 09
-</br></br>
+<br>
result 10
<table border="1" cellpadding="10" cellspacing="10">
<tr>
@@ -71,5 +88,22 @@
</tr>
</table>
result 15
+<div class="transform">
+<div style="height:0px">
+ <div style="float:left;">
+ result 16
+ </div>
+</div>
+</div>
+<div class="relative-overflow">
+ <div class="margin-overflow">
+ result 17
+ <div class="absolute">result 18</div>
+ </div>
+</div>
+<!-- Tests that will not yield matches should go below this line. -->
+<select>
+<option>result 19</option>
+</select>
</body>
</html>
