Changeset 126204 in webkit


Ignore:
Timestamp:
Aug 21, 2012 4:06:06 PM (12 years ago)
Author:
leandrogracia@chromium.org
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.
Location:
trunk/Source/WebKit/chromium
Files:
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebKit/chromium/ChangeLog

    r126202 r126204  
     12012-08-21  Leandro Gracia Gil  <leandrogracia@chromium.org>
     2
     3        [Chromium] Find-in-page coordinates should use containingBlock
     4        https://bugs.webkit.org/show_bug.cgi?id=94343
     5
     6        Reviewed by Julien Chaffraix.
     7
     8        The current implementation uses the container method to climb the render tree.
     9        However, it would be more correct and convenient to use containingBlock instead.
     10        Also, this patch introduces new tests for find-in-page coordinates in tables.
     11
     12        * src/FindInPageCoordinates.cpp:
     13        (WebKit::toNormalizedRect): use containingBlock and get rid of the output parameter as it's not required now.
     14        (WebKit::findInPageRectFromAbsoluteRect): use containingBlock introduce some simplifications.
     15        * tests/WebFrameTest.cpp: add new tests for tables.
     16        * tests/data/find_in_page.html:
     17        * tests/data/find_in_page_frame.html: new tests for tables.
     18
    1192012-08-21  Alexandre Elias  <aelias@google.com>
    220
  • trunk/Source/WebKit/chromium/src/FindInPageCoordinates.cpp

    r126074 r126204  
    3939#include "Node.h"
    4040#include "Range.h"
     41#include "RenderBlock.h"
    4142#include "RenderBox.h"
    4243#include "RenderObject.h"
     
    4950namespace WebKit {
    5051
    51 static FloatRect toNormalizedRect(const FloatRect& absoluteRect, const RenderObject* renderer, FloatRect& containerBoundingBox)
     52static FloatRect toNormalizedRect(const FloatRect& absoluteRect, const RenderObject* renderer)
    5253{
    5354    ASSERT(renderer);
    5455
    55     const RenderObject* container = renderer->container();
    56     if (!container) {
    57         containerBoundingBox = FloatRect();
     56    const RenderBlock* container = renderer->containingBlock();
     57    ASSERT(container || renderer->isRenderView());
     58    if (!container)
    5859        return FloatRect();
    59     }
    6060
    61     FloatRect normalizedRect = absoluteRect;
    62     FloatRect containerRect = container->absoluteBoundingBoxRect();
    63     containerBoundingBox = containerRect;
     61    // We want to normalize by the max layout overflow size instead of only the visible bounding box.
     62    // Quads and their enclosing bounding boxes need to be used in order to keep results transform-friendly.
     63    FloatPoint scrolledOrigin;
    6464
    65     // For RenderBoxes we want to normalize by the max layout overflow size instead of only the visible bounding box.
    66     // Quads and their enclosing bounding boxes need to be used in order to keep results transform-friendly.
    67     if (container->isBox()) {
    68         const RenderBox* containerBox = toRenderBox(container);
    69         FloatPoint scrolledOrigin;
     65    // For overflow:scroll we need to get where the actual origin is independently of the scroll.
     66    if (container->hasOverflowClip())
     67        scrolledOrigin = -IntPoint(container->scrolledContentOffset());
    7068
    71         // For overflow:scroll we need to get where the actual origin is independently of the scroll.
    72         if (container->hasOverflowClip())
    73             scrolledOrigin = -IntPoint(containerBox->scrolledContentOffset());
    74 
    75         FloatRect overflowRect(scrolledOrigin, containerBox->maxLayoutOverflow());
    76         containerRect = containerBox->localToAbsoluteQuad(FloatQuad(overflowRect), false).enclosingBoundingBox();
    77     }
     69    FloatRect overflowRect(scrolledOrigin, container->maxLayoutOverflow());
     70    FloatRect containerRect = container->localToAbsoluteQuad(FloatQuad(overflowRect), false).enclosingBoundingBox();
    7871
    7972    if (containerRect.isEmpty())
     
    8275    // Make the coordinates relative to the container enclosing bounding box.
    8376    // Since we work with rects enclosing quad unions this is still transform-friendly.
     77    FloatRect normalizedRect = absoluteRect;
    8478    normalizedRect.moveBy(-containerRect.location());
    8579
     
    9084
    9185    normalizedRect.scale(1 / containerRect.width(), 1 / containerRect.height());
    92 
    9386    return normalizedRect;
    9487}
    9588
    96 FloatRect findInPageRectFromAbsoluteRect(const FloatRect& inputRect, const RenderObject* renderer)
     89FloatRect findInPageRectFromAbsoluteRect(const FloatRect& inputRect, const RenderObject* baseRenderer)
    9790{
    98     if (!renderer || inputRect.isEmpty())
     91    if (!baseRenderer || inputRect.isEmpty())
    9992        return FloatRect();
    10093
    101     // Normalize the input rect to its container, saving the container bounding box for the incoming loop.
    102     FloatRect rendererBoundingBox;
    103     FloatRect normalizedRect = toNormalizedRect(inputRect, renderer, rendererBoundingBox);
    104     renderer = renderer->container();
     94    // Normalize the input rect to its container block.
     95    FloatRect normalizedRect = toNormalizedRect(inputRect, baseRenderer);
    10596
    10697    // Go up across frames.
    107     while (renderer) {
     98    for (const RenderObject* renderer = baseRenderer->containingBlock(); renderer; ) {
    10899
    109100        // Go up the render tree until we reach the root of the current frame (the RenderView).
    110         for (const RenderObject* container = renderer->container(); container; renderer = container, container = container->container()) {
     101        for (const RenderBlock* container = renderer->containingBlock(); container;
     102                renderer = container, container = container->containingBlock()) {
    111103
    112             // Compose the normalized rects. The absolute bounding box of the container is calculated in toNormalizedRect
    113             // and can be reused for the next iteration of the loop.
    114             FloatRect normalizedBoxRect = toNormalizedRect(rendererBoundingBox, renderer, rendererBoundingBox);
     104            // Compose the normalized rects.
     105            FloatRect normalizedBoxRect = toNormalizedRect(renderer->absoluteBoundingBoxRect(), renderer);
    115106            normalizedRect.scale(normalizedBoxRect.width(), normalizedBoxRect.height());
    116107            normalizedRect.moveBy(normalizedBoxRect.location());
     
    123114        ASSERT(renderer->isRenderView());
    124115        renderer = renderer->frame() ? renderer->frame()->ownerRenderer() : 0;
    125 
    126         // Update the absolute coordinates to the new frame.
    127         if (renderer)
    128             rendererBoundingBox = renderer->absoluteBoundingBoxRect();
    129116    }
    130117
  • trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp

    r125581 r126204  
    892892    static const char* kFindString = "result";
    893893    static const int kFindIdentifier = 12345;
    894     static const int kNumResults = 10;
     894    static const int kNumResults = 16;
    895895
    896896    WebFindOptions options;
     
    919919        Range* result = mainFrame->activeMatchFrame()->activeMatch();
    920920        ASSERT_TRUE(result);
    921         result->setEnd(result->endContainer(), result->endOffset() + 2);
    922         EXPECT_EQ(result->text(), String::format("%s %d", kFindString, resultIndex));
     921        result->setEnd(result->endContainer(), result->endOffset() + 3);
     922        EXPECT_EQ(result->text(), String::format("%s %02d", kFindString, resultIndex));
    923923
    924924        // Verify that the expected match rect also matches the currently active match.
     
    950950    EXPECT_TRUE(webMatchRects[7].y < webMatchRects[8].y);
    951951    EXPECT_TRUE(webMatchRects[8].y < webMatchRects[9].y);
     952
     953    // Results 11, 12, 13 and 14 should be between results 10 and 15, as they are inside the table.
     954    EXPECT_TRUE(webMatchRects[11].y > webMatchRects[10].y);
     955    EXPECT_TRUE(webMatchRects[12].y > webMatchRects[10].y);
     956    EXPECT_TRUE(webMatchRects[13].y > webMatchRects[10].y);
     957    EXPECT_TRUE(webMatchRects[14].y > webMatchRects[10].y);
     958    EXPECT_TRUE(webMatchRects[11].y < webMatchRects[15].y);
     959    EXPECT_TRUE(webMatchRects[12].y < webMatchRects[15].y);
     960    EXPECT_TRUE(webMatchRects[13].y < webMatchRects[15].y);
     961    EXPECT_TRUE(webMatchRects[14].y < webMatchRects[15].y);
     962
     963    // Result 11 should be above 12, 13 and 14 as it's in the table header.
     964    EXPECT_TRUE(webMatchRects[11].y < webMatchRects[12].y);
     965    EXPECT_TRUE(webMatchRects[11].y < webMatchRects[13].y);
     966    EXPECT_TRUE(webMatchRects[11].y < webMatchRects[14].y);
     967
     968    // Result 11 should also be right to 12, 13 and 14 because of the colspan.
     969    EXPECT_TRUE(webMatchRects[11].x > webMatchRects[12].x);
     970    EXPECT_TRUE(webMatchRects[11].x > webMatchRects[13].x);
     971    EXPECT_TRUE(webMatchRects[11].x > webMatchRects[14].x);
     972
     973    // Result 12 should be left to results 11, 13 and 14 in the table layout.
     974    EXPECT_TRUE(webMatchRects[12].x < webMatchRects[11].x);
     975    EXPECT_TRUE(webMatchRects[12].x < webMatchRects[13].x);
     976    EXPECT_TRUE(webMatchRects[12].x < webMatchRects[14].x);
     977
     978    // Results 13, 12 and 14 should be one above the other in that order because of the rowspan
     979    // and vertical-align: middle by default.
     980    EXPECT_TRUE(webMatchRects[13].y < webMatchRects[12].y);
     981    EXPECT_TRUE(webMatchRects[12].y < webMatchRects[14].y);
    952982
    953983    // Resizing should update the rects version.
  • trunk/Source/WebKit/chromium/tests/data/find_in_page.html

    r125379 r126204  
    66<body>
    77This a find-in-page match rect test.</br>
    8 result 0</br>
     8result 00</br>
    99<iframe src="find_in_page_frame.html" height="300" scrolling="yes"></iframe>
    1010</br>
    11 result 1
     11result 01
    1212</body>
    1313</html>
  • trunk/Source/WebKit/chromium/tests/data/find_in_page_frame.html

    r125379 r126204  
    3737</br></br>
    3838</br></br>
    39 result 2
     39result 02
    4040<div class="transform">
    41 result 3
     41result 03
    4242</div>
    43 result 4
     43result 04
    4444<div class="fixed">
    45 result 5
     45result 05
    4646</div>
    47 result 6
     47result 06
    4848<div class="scroll">
    49 result 7
     49result 07
    5050Foo bar.
    5151</br></br>
    5252</br></br>
    5353</br></br>
    54 result 8
     54result 08
    5555</div>
    56 result 9
     56result 09
     57</br></br>
     58result 10
     59<table border="1" cellpadding="10" cellspacing="10">
     60<tr>
     61  <th>Foo</th>
     62  <th>Bar</th>
     63  <th>result 11</th>
     64</tr>
     65<tr>
     66  <td rowspan="2">result 12</td>
     67  <td colspan="2">result 13</td>
     68</tr>
     69<tr>
     70  <td colspan="2">result 14</td>
     71</tr>
     72</table>
     73result 15
    5774</body>
    5875</html>
Note: See TracChangeset for help on using the changeset viewer.