Changeset 135841 in webkit


Ignore:
Timestamp:
Nov 27, 2012 3:03:23 AM (11 years ago)
Author:
allan.jensen@digia.com
Message:

Incorrect rect-based hit-test result when hit-test region includes culled inlines
​https://bugs.webkit.org/show_bug.cgi?id=88376

Patch by Allan Sandfeld Jensen <​allan.jensen@nokia.com> on 2012-09-17
Reviewed by Dave Hyatt.

Source/WebCore:

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.

Tests: fast/dom/nodesFromRect/nodesFromRect-culled-inlines.html

fast/dom/nodesFromRect/nodesFromRect-culled-inline-with-linebreak.html

  • rendering/HitTestResult.cpp:

(WebCore::HitTestLocation::HitTestLocation):
(WebCore::HitTestResult::addNodeToRectBasedTestResult):

  • rendering/HitTestResult.h:

(HitTestLocation):

  • rendering/InlineFlowBox.cpp:

(WebCore::InlineFlowBox::nodeAtPoint):

  • rendering/RenderInline.cpp:

(WebCore::RenderInline::hitTestCulledInline):

  • rendering/RenderInline.h:

(RenderInline):

LayoutTests:

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 two new tests that does
test culled inlines.

  • fast/dom/nodesFromRect/nodesFromRect-culled-inline-with-linebreak-expected.txt: Copied from LayoutTests/fast/dom/nodesFromRect/nodesFromRect-culled-inlines-expected.txt.
  • fast/dom/nodesFromRect/nodesFromRect-culled-inline-with-linebreak.html: Added.
  • 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):
(nodesFromRectAsString):

Location:
trunk
Files:
3 added
10 edited
1 copied

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r135840 r135841  
     12012-09-17  Allan Sandfeld Jensen  <allan.jensen@nokia.com>
     2
     3        Incorrect rect-based hit-test result when hit-test region includes culled inlines
     4        https://bugs.webkit.org/show_bug.cgi?id=88376
     5
     6        Reviewed by Dave Hyatt.
     7
     8        Renames the existing nodesFromRect-culled-inlines.html test to nodesFromRect-inline-image.html,
     9        because it did not test any culled inlines anymore, and replace it with two new tests that does
     10        test culled inlines.
     11
     12        * fast/dom/nodesFromRect/nodesFromRect-culled-inline-with-linebreak-expected.txt: Copied from LayoutTests/fast/dom/nodesFromRect/nodesFromRect-culled-inlines-expected.txt.
     13        * fast/dom/nodesFromRect/nodesFromRect-culled-inline-with-linebreak.html: Added.
     14        * fast/dom/nodesFromRect/nodesFromRect-culled-inlines-expected.txt:
     15        * fast/dom/nodesFromRect/nodesFromRect-culled-inlines.html:
     16        * fast/dom/nodesFromRect/nodesFromRect-inline-image-expected.txt: Added.
     17        * fast/dom/nodesFromRect/nodesFromRect-inline-image.html: Added.
     18        * fast/dom/nodesFromRect/resources/nodesFromRect.js:
     19        (checkRect):
     20        (nodesFromRectAsString):
     21
    1222012-11-27  Kent Tamura  <tkent@chromium.org>
    223

  • trunk/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-culled-inline-with-linebreak-expected.txt

    r135840 r135841  
    1   PASS All correct nodes found for rect
    2 PASS All correct nodes found for rect
    3 PASS All correct nodes found for rect
     1Document::nodesFromRect : culled inline with line-break - bug 88376
     2
     3On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
     4
     5
    46PASS All correct nodes found for rect
    57PASS All correct nodes found for rect

  • trunk/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-culled-inlines-expected.txt

    r122473 r135841  
    1   PASS All correct nodes found for rect
     1Document::nodesFromRect : culled inlines - bug 88376
     2
     3On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
     4
     5
     6PASS All correct nodes found for rect
     7PASS All correct nodes found for rect
    28PASS All correct nodes found for rect
    39PASS All correct nodes found for rect

  • trunk/LayoutTests/fast/dom/nodesFromRect/nodesFromRect-culled-inlines.html

    r122473 r135841  
    22<html>
    33<head>
    4   <title>Document::nodesFromRect test - bug 85849</title>
    5   <script src="../../js/resources/js-test-pre.js"></script>
    6   <script src="resources/nodesFromRect.js"></script>
    7   <script type="application/javascript">
    8     function runTest()
    9     {
    10       var e = {};
     4    <title>Document::nodesFromRect : culled inlines - bug 88376</title>
     5    <script src="../../js/resources/js-test-pre.js"></script>
     6    <script src="resources/nodesFromRect.js"></script>
     7    <style>
     8        #sandbox {
     9            position: absolute;
     10            left: 0px;
     11            top: 0px;
     12            width: 400px;
     13            height: 200px;
     14        }
     15        #sandbox p { font: 16px Ahem; }
     16    </style>
     17</head>
     18<body id="body">
     19    <div id=sandbox>
     20        <p><span id=culledinline><span id=wordinline1>word1</span>   <span id=wordinline2>word2</span></span>   <span id=wordinline3>word3</span></p>
     21    </div>
    1122
    12       // Set up shortcut access to elements
    13       e['html'] = document.getElementsByTagName("html")[0];
    14       ['body', 'span', 'img'].forEach(function(a) {
    15         e[a] = document.getElementById(a);
    16       });
     23    <p id="description"></p>
     24    <span id="console"></span>
     25    <script type="application/javascript">
     26        function runTest()
     27        {
     28            description(document.title);
     29            window.scrollTo(0, 0);
     30            /* Rect based test over word1 only. */
     31            checkRect(30, 19, 8, 8, "'word1'");
     32            /* Rect based test over the word2 only. */
     33            checkRect(126, 19, 8, 8, "'word2'");
     34            /* Rect based test over the word3 only. */
     35            checkRect(222, 19, 8, 8, "'word3'");
     36            /* Rect based test between word1 and word2. */
     37            checkRect(84, 19, 8, 8, "'   '");
     38            /* Rect based test over and outside word1. */
     39            checkRect(70, 19, 20, 8, "'   ', 'word1', SPAN#wordinline1, SPAN#culledinline");
     40            /* Rect based test over word1 and word2. */
     41            checkRect(70, 19, 40, 8, "'word2', SPAN#wordinline2, '   ', 'word1', SPAN#wordinline1, SPAN#culledinline");
     42            /* Rect based test over word2 and word3. */
     43            checkRect(170, 19, 40, 8, "'word3', SPAN#wordinline3, '   ', 'word2', SPAN#wordinline2, SPAN#culledinline, P");
    1744
    18       window.scrollTo(0, 0);
    19 
    20       /* Point based test over the img only. */
    21       check(20, 20, 0, 0, 0, 0, [e.img]);
    22       /* Rect based test over the img only. */
    23       check(20, 20, 5, 5, 5, 5, [e.img]);
    24 
    25       /* Note that for the tests below, the img bounds are considered to be (99, 99). */
    26       /* Point based test over the img and the span. */
    27       check(0, 99, 0, 0, 0, 0, [e.img]);
    28       /* Rect based test over the img and the span with the img fully covering the hit region. */
    29       check(0, 98, 0, 1, 1, 0, [e.img]);
    30       /* Rect based test over the img and the span with the img not fully covering the hit region. */
    31       /* FIXME: This fails due to: https://bugs.webkit.org/show_bug.cgi?id=88376 */
    32       check(0, 98, 0, 1, 2, 0, [e.img, e.span]);
    33       /* Rect based test over the entire img. */
    34       check(0, 0, 0, 99, 99, 0, [e.img]);
    35     }
    36   </script>
    37 </head>
    38 <body id="body" style="padding: 0; margin: 0;">
    39   <span id="span" style="margin: 0; padding: 0; font-size:36px">
    40     <img id="img" width="100" height="100" style="background-color: black; margin: 0; padding: 0;" />
    41   </span>
    42 
    43   <span id="console" style="position: absolute; top: 150px;"></span>
    44   <script> runTest();</script>
    45   <script src="../../js/resources/js-test-post.js"></script>
     45            document.getElementById('sandbox').style.display = 'none';
     46        }
     47        runTest();
     48    </script>
     49    <script src="../../js/resources/js-test-post.js"></script>
    4650</body>
    4751</html>

  • trunk/LayoutTests/fast/dom/nodesFromRect/resources/nodesFromRect.js

    r128677 r135841  
    6868}
    6969
     70function checkRect(left, top, width, height, expectedNodeString, doc)
     71{
     72    if (!window.internals)
     73        return;
     74
     75    if (height <=0 || width <= 0)
     76        return;
     77
     78    if (!doc)
     79        doc = document;
     80
     81    var topPadding = height / 2;
     82    var leftPadding =  width / 2;
     83    // FIXME: When nodesFromRect is changed to not add 1 to width and height, remove the correction here.
     84    var bottomPadding = (height - 1) - topPadding;
     85    var rightPadding = (width - 1) - leftPadding;
     86
     87    var nodeString = nodesFromRectAsString(doc, left + leftPadding, top + topPadding, topPadding, rightPadding, bottomPadding, leftPadding);
     88
     89    if (nodeString == expectedNodeString) {
     90        testPassed("All correct nodes found for rect");
     91    } else {
     92        testFailed("NodesFromRect should be [" + expectedNodeString + "] was [" + nodeString + "]");
     93    }
     94}
     95
     96function nodesFromRectAsString(doc, x, y, topPadding, rightPadding, bottomPadding, leftPadding)
     97{
     98    var nodeString = "";
     99    var nodes = internals.nodesFromRect(doc, x, y, topPadding, rightPadding, bottomPadding, leftPadding, true /* ignoreClipping */, false /* allow shadow content */);
     100    if (!nodes)
     101        return nodeString;
     102
     103    for (var i = 0; i < nodes.length; i++) {
     104        if (nodes[i].nodeType == 1) {
     105            nodeString += nodes[i].nodeName;
     106            if (nodes[i].id)
     107                nodeString += '#' + nodes[i].id;
     108            else if (nodes[i].class) {
     109                nodeString += '.' + nodes[i].class;
     110            }
     111        } else if (nodes[i].nodeType == 3) {
     112            nodeString += "'" + nodes[i].data + "'";
     113        } else {
     114            continue;
     115        }
     116        if (i + 1 < nodes.length) {
     117            nodeString += ", ";
     118        }
     119    }
     120    return nodeString;
     121}
     122
     123
    70124function getCenterFor(element)
    71125{

  • trunk/Source/WebCore/ChangeLog

    r135839 r135841  
     12012-09-17  Allan Sandfeld Jensen  <allan.jensen@nokia.com>
     2
     3        Incorrect rect-based hit-test result when hit-test region includes culled inlines
     4        https://bugs.webkit.org/show_bug.cgi?id=88376
     5
     6        Reviewed by Dave Hyatt.
     7
     8        Move the handling of culled inlines from HitTestResult::addNodeToRectBasedTestResult to
     9        InlineFlowBox::nodeAtPoint. This makes it possible to fix a number of bugs with how
     10        culled inlines were handled. They are now checked after all their children, and may
     11        terminate area-based hit-testing if they contain the whole area.
     12
     13        Tests: fast/dom/nodesFromRect/nodesFromRect-culled-inlines.html
     14               fast/dom/nodesFromRect/nodesFromRect-culled-inline-with-linebreak.html
     15
     16        * rendering/HitTestResult.cpp:
     17        (WebCore::HitTestLocation::HitTestLocation):
     18        (WebCore::HitTestResult::addNodeToRectBasedTestResult):
     19        * rendering/HitTestResult.h:
     20        (HitTestLocation):
     21        * rendering/InlineFlowBox.cpp:
     22        (WebCore::InlineFlowBox::nodeAtPoint):
     23        * rendering/RenderInline.cpp:
     24        (WebCore::RenderInline::hitTestCulledInline):
     25        * rendering/RenderInline.h:
     26        (RenderInline):
     27
    1282012-11-27  Kenneth Rohde Christiansen  <kenneth@webkit.org>
    229

  • trunk/Source/WebCore/rendering/HitTestResult.cpp

    r135710 r135841  
    105105    , m_transformedPoint(other.m_transformedPoint)
    106106    , m_transformedRect(other.m_transformedRect)
    107     , m_region(region)
     107    , m_region(region ? region : other.m_region)
    108108    , m_isRectBased(other.m_isRectBased)
    109109    , m_isRectilinear(other.m_isRectilinear)
    … …  
    701701
    702702    bool regionFilled = rect.contains(locationInContainer.boundingBox());
    703     // FIXME: This code (incorrectly) attempts to correct for culled inline nodes. See https://bugs.webkit.org/show_bug.cgi?id=85849.
    704     if (node->renderer()->isInline() && !regionFilled) {
    705         for (RenderObject* curr = node->renderer()->parent(); curr; curr = curr->parent()) {
    706             if (!curr->isRenderInline())
    707                 break;
    708 
    709             // We need to make sure the nodes for culled inlines get included.
    710             RenderInline* currInline = toRenderInline(curr);
    711             if (currInline->alwaysCreateLineBoxes())
    712                 break;
    713 
    714             if (currInline->visibleToHitTesting() && currInline->node())
    715                 mutableRectBasedTestResult().add(currInline->node()->shadowAncestorNode());
    716         }
    717     }
    718703    return !regionFilled;
    719704}
    … …  
    736721
    737722    bool regionFilled = rect.contains(locationInContainer.boundingBox());
    738     // FIXME: This code (incorrectly) attempts to correct for culled inline nodes. See https://bugs.webkit.org/show_bug.cgi?id=85849.
    739     if (node->renderer()->isInline() && !regionFilled) {
    740         for (RenderObject* curr = node->renderer()->parent(); curr; curr = curr->parent()) {
    741             if (!curr->isRenderInline())
    742                 break;
    743 
    744             // We need to make sure the nodes for culled inlines get included.
    745             RenderInline* currInline = toRenderInline(curr);
    746             if (currInline->alwaysCreateLineBoxes())
    747                 break;
    748 
    749             if (currInline->visibleToHitTesting() && currInline->node())
    750                 mutableRectBasedTestResult().add(currInline->node()->shadowAncestorNode());
    751         }
    752     }
    753723    return !regionFilled;
    754724}

  • trunk/Source/WebCore/rendering/HitTestResult.h

    r135710 r135841  
    5757    HitTestLocation(const LayoutPoint& centerPoint, unsigned topPadding, unsigned rightPadding, unsigned bottomPadding, unsigned leftPadding);
    5858    // Make a copy the HitTestLocation in a new region by applying given offset to internal point and area.
    59     HitTestLocation(const HitTestLocation&, const LayoutSize& offset, RenderRegion*);
     59    HitTestLocation(const HitTestLocation&, const LayoutSize& offset, RenderRegion* = 0);
    6060    HitTestLocation(const HitTestLocation&);
    6161    ~HitTestLocation();

  • trunk/Source/WebCore/rendering/InlineFlowBox.cpp

    r135750 r135841  
    982982
    983983    // Check children first.
     984    // 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.
     985    RenderObject* culledParent = 0;
    984986    for (InlineBox* curr = lastChild(); curr; curr = curr->prevOnLine()) {
    985         if ((curr->renderer()->isText() || !curr->boxModelObject()->hasSelfPaintingLayer()) && curr->nodeAtPoint(request, result, locationInContainer, accumulatedOffset, lineTop, lineBottom)) {
    986             renderer()->updateHitTestResult(result, locationInContainer.point() - toLayoutSize(accumulatedOffset));
     987        if (curr->renderer()->isText() || !curr->boxModelObject()->hasSelfPaintingLayer()) {
     988            RenderObject* newParent = 0;
     989            // Culled parents are only relevant for area-based hit-tests, so ignore it in point-based ones.
     990            if (locationInContainer.isRectBasedTest()) {
     991                newParent = curr->renderer()->parent();
     992                if (newParent == renderer())
     993                    newParent = 0;
     994            }
     995            // Check the culled parent after all its children have been checked, to do this we wait until
     996            // we are about to test an element with a different parent.
     997            if (newParent != culledParent) {
     998                if (!newParent || !newParent->isDescendantOf(culledParent)) {
     999                    while (culledParent && culledParent != renderer() && culledParent != newParent) {
     1000                        if (culledParent->isRenderInline() && toRenderInline(culledParent)->hitTestCulledInline(request, result, locationInContainer, accumulatedOffset))
     1001                            return true;
     1002                        culledParent = culledParent->parent();
     1003                    }
     1004                }
     1005                culledParent = newParent;
     1006            }
     1007            if (curr->nodeAtPoint(request, result, locationInContainer, accumulatedOffset, lineTop, lineBottom)) {
     1008                renderer()->updateHitTestResult(result, locationInContainer.point() - toLayoutSize(accumulatedOffset));
     1009                return true;
     1010            }
     1011        }
     1012    }
     1013    // Check any culled ancestor of the final children tested.
     1014    while (culledParent && culledParent != renderer()) {
     1015        if (culledParent->isRenderInline() && toRenderInline(culledParent)->hitTestCulledInline(request, result, locationInContainer, accumulatedOffset))
    9871016            return true;
    988         }
     1017        culledParent = culledParent->parent();
    9891018    }
    9901019

  • trunk/Source/WebCore/rendering/RenderInline.cpp

    r135025 r135841  
    775775}
    776776
     777namespace {
     778
     779class HitTestCulledInlinesGeneratorContext {
     780public:
     781    HitTestCulledInlinesGeneratorContext(Region& region, const HitTestLocation& location) : m_intersected(false), m_region(region), m_location(location) { }
     782    void operator()(const FloatRect& rect)
     783    {
     784        m_intersected = m_intersected || m_location.intersects(rect);
     785        m_region.unite(enclosingIntRect(rect));
     786    }
     787    bool intersected() const { return m_intersected; }
     788private:
     789    bool m_intersected;
     790    Region& m_region;
     791    const HitTestLocation& m_location;
     792};
     793
     794} // unnamed namespace
     795
     796bool RenderInline::hitTestCulledInline(const HitTestRequest& request, HitTestResult& result, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset)
     797{
     798    ASSERT(result.isRectBasedTest() && !alwaysCreateLineBoxes());
     799    if (!visibleToHitTesting())
     800        return false;
     801
     802    HitTestLocation tmpLocation(locationInContainer, -toLayoutSize(accumulatedOffset));
     803
     804    Region regionResult;
     805    HitTestCulledInlinesGeneratorContext context(regionResult, tmpLocation);
     806    generateCulledLineBoxRects(context, this);
     807
     808    if (context.intersected()) {
     809        updateHitTestResult(result, tmpLocation.point());
     810        // We can not use addNodeToRectBasedTestResult to determine if we fully enclose the hit-test area
     811        // because it can only handle rectangular targets.
     812        result.addNodeToRectBasedTestResult(node(), request, locationInContainer);
     813        return regionResult.contains(enclosingIntRect(tmpLocation.boundingBox()));
     814    }
     815    return false;
     816}
     817
    777818VisiblePosition RenderInline::positionForPoint(const LayoutPoint& point)
    778819{

  • trunk/Source/WebCore/rendering/RenderInline.h

    r133845 r135841  
    9090    virtual LayoutRect localCaretRect(InlineBox*, int, LayoutUnit* extraWidthToEndOfLine) OVERRIDE;
    9191
     92    bool hitTestCulledInline(const HitTestRequest&, HitTestResult&, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset);
     93
    9294protected:
    9395    virtual void willBeDestroyed();
Note: See TracChangeset for help on using the changeset viewer.