- Revision
- 126204
- Author
- [email protected]
- Date
- 2012-08-21 16:06:06 -0700 (Tue, 21 Aug 2012)
Log Message
[Chromium] Find-in-page coordinates should use containingBlock
https://bugs.webkit.org/show_bug.cgi?id=94343
Reviewed by Julien Chaffraix.
The current implementation uses the container method to climb the render tree.
However, it would be more correct and convenient to use containingBlock instead.
Also, this patch introduces new tests for find-in-page coordinates in tables.
* src/FindInPageCoordinates.cpp:
(WebKit::toNormalizedRect): use containingBlock and get rid of the output parameter as it's not required now.
(WebKit::findInPageRectFromAbsoluteRect): use containingBlock introduce some simplifications.
* tests/WebFrameTest.cpp: add new tests for tables.
* tests/data/find_in_page.html:
* tests/data/find_in_page_frame.html: new tests for tables.
Modified Paths
Diff
Modified: trunk/Source/WebKit/chromium/ChangeLog (126203 => 126204)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-08-21 23:04:19 UTC (rev 126203)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-08-21 23:06:06 UTC (rev 126204)
@@ -1,3 +1,21 @@
+2012-08-21 Leandro Gracia Gil <[email protected]>
+
+ [Chromium] Find-in-page coordinates should use containingBlock
+ https://bugs.webkit.org/show_bug.cgi?id=94343
+
+ Reviewed by Julien Chaffraix.
+
+ The current implementation uses the container method to climb the render tree.
+ However, it would be more correct and convenient to use containingBlock instead.
+ Also, this patch introduces new tests for find-in-page coordinates in tables.
+
+ * src/FindInPageCoordinates.cpp:
+ (WebKit::toNormalizedRect): use containingBlock and get rid of the output parameter as it's not required now.
+ (WebKit::findInPageRectFromAbsoluteRect): use containingBlock introduce some simplifications.
+ * tests/WebFrameTest.cpp: add new tests for tables.
+ * tests/data/find_in_page.html:
+ * tests/data/find_in_page_frame.html: new tests for tables.
+
2012-08-21 Alexandre Elias <[email protected]>
[chromium] Add software bitmap resources to CCResourceProvider
Modified: trunk/Source/WebKit/chromium/src/FindInPageCoordinates.cpp (126203 => 126204)
--- trunk/Source/WebKit/chromium/src/FindInPageCoordinates.cpp 2012-08-21 23:04:19 UTC (rev 126203)
+++ trunk/Source/WebKit/chromium/src/FindInPageCoordinates.cpp 2012-08-21 23:06:06 UTC (rev 126204)
@@ -38,6 +38,7 @@
#include "IntPoint.h"
#include "Node.h"
#include "Range.h"
+#include "RenderBlock.h"
#include "RenderBox.h"
#include "RenderObject.h"
#include "RenderPart.h"
@@ -48,39 +49,32 @@
namespace WebKit {
-static FloatRect toNormalizedRect(const FloatRect& absoluteRect, const RenderObject* renderer, FloatRect& containerBoundingBox)
+static FloatRect toNormalizedRect(const FloatRect& absoluteRect, const RenderObject* renderer)
{
ASSERT(renderer);
- const RenderObject* container = renderer->container();
- if (!container) {
- containerBoundingBox = FloatRect();
+ const RenderBlock* container = renderer->containingBlock();
+ ASSERT(container || renderer->isRenderView());
+ if (!container)
return FloatRect();
- }
- FloatRect normalizedRect = absoluteRect;
- FloatRect containerRect = container->absoluteBoundingBoxRect();
- containerBoundingBox = containerRect;
-
- // For RenderBoxes we want to normalize by the max layout overflow size instead of only the visible bounding box.
+ // We want to normalize by the max layout overflow size instead of only the visible bounding box.
// Quads and their enclosing bounding boxes need to be used in order to keep results transform-friendly.
- if (container->isBox()) {
- const RenderBox* containerBox = toRenderBox(container);
- FloatPoint scrolledOrigin;
+ FloatPoint scrolledOrigin;
- // For overflow:scroll we need to get where the actual origin is independently of the scroll.
- if (container->hasOverflowClip())
- scrolledOrigin = -IntPoint(containerBox->scrolledContentOffset());
+ // For overflow:scroll we need to get where the actual origin is independently of the scroll.
+ if (container->hasOverflowClip())
+ scrolledOrigin = -IntPoint(container->scrolledContentOffset());
- FloatRect overflowRect(scrolledOrigin, containerBox->maxLayoutOverflow());
- containerRect = containerBox->localToAbsoluteQuad(FloatQuad(overflowRect), false).enclosingBoundingBox();
- }
+ FloatRect overflowRect(scrolledOrigin, container->maxLayoutOverflow());
+ FloatRect containerRect = container->localToAbsoluteQuad(FloatQuad(overflowRect), false).enclosingBoundingBox();
if (containerRect.isEmpty())
return FloatRect();
// Make the coordinates relative to the container enclosing bounding box.
// Since we work with rects enclosing quad unions this is still transform-friendly.
+ FloatRect normalizedRect = absoluteRect;
normalizedRect.moveBy(-containerRect.location());
// Fixed positions do not make sense in this coordinate system, but need to leave consistent tickmarks.
@@ -89,29 +83,26 @@
normalizedRect.move(-toRenderView(container)->frameView()->scrollOffsetForFixedPosition());
normalizedRect.scale(1 / containerRect.width(), 1 / containerRect.height());
-
return normalizedRect;
}
-FloatRect findInPageRectFromAbsoluteRect(const FloatRect& inputRect, const RenderObject* renderer)
+FloatRect findInPageRectFromAbsoluteRect(const FloatRect& inputRect, const RenderObject* baseRenderer)
{
- if (!renderer || inputRect.isEmpty())
+ if (!baseRenderer || inputRect.isEmpty())
return FloatRect();
- // Normalize the input rect to its container, saving the container bounding box for the incoming loop.
- FloatRect rendererBoundingBox;
- FloatRect normalizedRect = toNormalizedRect(inputRect, renderer, rendererBoundingBox);
- renderer = renderer->container();
+ // Normalize the input rect to its container block.
+ FloatRect normalizedRect = toNormalizedRect(inputRect, baseRenderer);
// Go up across frames.
- while (renderer) {
+ for (const RenderObject* renderer = baseRenderer->containingBlock(); renderer; ) {
// Go up the render tree until we reach the root of the current frame (the RenderView).
- for (const RenderObject* container = renderer->container(); container; renderer = container, container = container->container()) {
+ for (const RenderBlock* container = renderer->containingBlock(); container;
+ renderer = container, container = container->containingBlock()) {
- // Compose the normalized rects. The absolute bounding box of the container is calculated in toNormalizedRect
- // and can be reused for the next iteration of the loop.
- FloatRect normalizedBoxRect = toNormalizedRect(rendererBoundingBox, renderer, rendererBoundingBox);
+ // Compose the normalized rects.
+ FloatRect normalizedBoxRect = toNormalizedRect(renderer->absoluteBoundingBoxRect(), renderer);
normalizedRect.scale(normalizedBoxRect.width(), normalizedBoxRect.height());
normalizedRect.moveBy(normalizedBoxRect.location());
@@ -122,10 +113,6 @@
// Jump to the renderer owning the frame, if any.
ASSERT(renderer->isRenderView());
renderer = renderer->frame() ? renderer->frame()->ownerRenderer() : 0;
-
- // Update the absolute coordinates to the new frame.
- if (renderer)
- rendererBoundingBox = renderer->absoluteBoundingBoxRect();
}
return normalizedRect;
Modified: trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp (126203 => 126204)
--- trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp 2012-08-21 23:04:19 UTC (rev 126203)
+++ trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp 2012-08-21 23:06:06 UTC (rev 126204)
@@ -891,7 +891,7 @@
static const char* kFindString = "result";
static const int kFindIdentifier = 12345;
- static const int kNumResults = 10;
+ static const int kNumResults = 16;
WebFindOptions options;
WebString searchText = WebString::fromUTF8(kFindString);
@@ -918,8 +918,8 @@
// Check that the find result ordering matches with our expectations.
Range* result = mainFrame->activeMatchFrame()->activeMatch();
ASSERT_TRUE(result);
- result->setEnd(result->endContainer(), result->endOffset() + 2);
- EXPECT_EQ(result->text(), String::format("%s %d", kFindString, resultIndex));
+ result->setEnd(result->endContainer(), result->endOffset() + 3);
+ EXPECT_EQ(result->text(), String::format("%s %02d", kFindString, resultIndex));
// Verify that the expected match rect also matches the currently active match.
// Compare the enclosing rects to prevent precision issues caused by CSS transforms.
@@ -950,6 +950,36 @@
EXPECT_TRUE(webMatchRects[7].y < webMatchRects[8].y);
EXPECT_TRUE(webMatchRects[8].y < webMatchRects[9].y);
+ // Results 11, 12, 13 and 14 should be between results 10 and 15, as they are inside the table.
+ EXPECT_TRUE(webMatchRects[11].y > webMatchRects[10].y);
+ EXPECT_TRUE(webMatchRects[12].y > webMatchRects[10].y);
+ EXPECT_TRUE(webMatchRects[13].y > webMatchRects[10].y);
+ EXPECT_TRUE(webMatchRects[14].y > webMatchRects[10].y);
+ EXPECT_TRUE(webMatchRects[11].y < webMatchRects[15].y);
+ EXPECT_TRUE(webMatchRects[12].y < webMatchRects[15].y);
+ EXPECT_TRUE(webMatchRects[13].y < webMatchRects[15].y);
+ EXPECT_TRUE(webMatchRects[14].y < webMatchRects[15].y);
+
+ // Result 11 should be above 12, 13 and 14 as it's in the table header.
+ EXPECT_TRUE(webMatchRects[11].y < webMatchRects[12].y);
+ EXPECT_TRUE(webMatchRects[11].y < webMatchRects[13].y);
+ EXPECT_TRUE(webMatchRects[11].y < webMatchRects[14].y);
+
+ // Result 11 should also be right to 12, 13 and 14 because of the colspan.
+ EXPECT_TRUE(webMatchRects[11].x > webMatchRects[12].x);
+ EXPECT_TRUE(webMatchRects[11].x > webMatchRects[13].x);
+ EXPECT_TRUE(webMatchRects[11].x > webMatchRects[14].x);
+
+ // Result 12 should be left to results 11, 13 and 14 in the table layout.
+ EXPECT_TRUE(webMatchRects[12].x < webMatchRects[11].x);
+ EXPECT_TRUE(webMatchRects[12].x < webMatchRects[13].x);
+ EXPECT_TRUE(webMatchRects[12].x < webMatchRects[14].x);
+
+ // Results 13, 12 and 14 should be one above the other in that order because of the rowspan
+ // and vertical-align: middle by default.
+ EXPECT_TRUE(webMatchRects[13].y < webMatchRects[12].y);
+ EXPECT_TRUE(webMatchRects[12].y < webMatchRects[14].y);
+
// Resizing should update the rects version.
webView->resize(WebSize(800, 600));
webkit_support::RunAllPendingMessages();
Modified: trunk/Source/WebKit/chromium/tests/data/find_in_page.html (126203 => 126204)
--- trunk/Source/WebKit/chromium/tests/data/find_in_page.html 2012-08-21 23:04:19 UTC (rev 126203)
+++ trunk/Source/WebKit/chromium/tests/data/find_in_page.html 2012-08-21 23:06:06 UTC (rev 126204)
@@ -5,9 +5,9 @@
</head>
<body>
This a find-in-page match rect test.</br>
-result 0</br>
+result 00</br>
<iframe src="" height="300" scrolling="yes"></iframe>
</br>
-result 1
+result 01
</body>
</html>
Modified: trunk/Source/WebKit/chromium/tests/data/find_in_page_frame.html (126203 => 126204)
--- trunk/Source/WebKit/chromium/tests/data/find_in_page_frame.html 2012-08-21 23:04:19 UTC (rev 126203)
+++ trunk/Source/WebKit/chromium/tests/data/find_in_page_frame.html 2012-08-21 23:06:06 UTC (rev 126204)
@@ -36,23 +36,40 @@
</br></br>
</br></br>
</br></br>
-result 2
+result 02
<div class="transform">
-result 3
+result 03
</div>
-result 4
+result 04
<div class="fixed">
-result 5
+result 05
</div>
-result 6
+result 06
<div class="scroll">
-result 7
+result 07
Foo bar.
</br></br>
</br></br>
</br></br>
-result 8
+result 08
</div>
-result 9
+result 09
+</br></br>
+result 10
+<table border="1" cellpadding="10" cellspacing="10">
+<tr>
+ <th>Foo</th>
+ <th>Bar</th>
+ <th>result 11</th>
+</tr>
+<tr>
+ <td rowspan="2">result 12</td>
+ <td colspan="2">result 13</td>
+</tr>
+<tr>
+ <td colspan="2">result 14</td>
+</tr>
+</table>
+result 15
</body>
</html>
