⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 167210 in webkit


Ignore:
Timestamp:
Apr 13, 2014, 11:55:15 PM (12 years ago)
Author:
Darin Adler
Message:

REGRESSION (r158617): Find on Page can get stuck in a loop when the search string occurs in an <input> in a <fieldset>
​https://bugs.webkit.org/show_bug.cgi?id=126322

Reviewed by Ryosuke Niwa.

Source/WebCore:

  • dom/Element.cpp:

(WebCore::Element::canContainRangeEndPoint): Now returns false when the role of the element
is "img". This is the same rule that's hard-coded in isRenderReplacedElement for the same
reason. Need more test coverage to make sure this role feature works consistently.

  • dom/Element.h: Made canContainRangeEndPoint no longer inline since it's not just a

return statement any more.

  • dom/Position.cpp:

(WebCore::Position::isCandidate): Took out code that calls isRendererReplacedElement
that was added in r158617; not needed now that we updated canContainRangeEndPoint.

  • dom/Range.cpp:

(WebCore::Range::firstNode): Removed code here that called isRendererReplacedElement.
This was the wrong level to be adding editing logic, and there's a FIXME here to that
effect, which we are now deleting. This was the change that broke Find.

  • editing/TextIterator.cpp: Added a comment about the redundancy between the

isRendererReplacedElement and editingIgnoresContent functions.

  • html/HTMLHRElement.cpp:

(WebCore::HTMLHRElement::canContainRangeEndPoint): Call through to base class instead
of just returning true when we have child nodes. Lets Element::canContainRangeEndPoint
do its thing.

  • html/HTMLHRElement.h: Ditto.
  • html/HTMLObjectElement.cpp:

(WebCore::HTMLObjectElement::canContainRangeEndPoint): Call through to base class instead
of just returning true when we have fallback content. Lets Element::canContainRangeEndPoint
do its thing.

  • html/HTMLObjectElement.h: Ditto.
  • testing/Internals.cpp:

(WebCore::Internals::countMatchesForText): Set the limit to 1000 instead of infinite.

LayoutTests:

  • editing/text-iterator/count-matches-in-form-expected.txt: Added.
  • editing/text-iterator/count-matches-in-form.html: Added.
  • fast/text/window-find.html: Tweaked the test a bit, making it a little easier to

see if the test hasn't even run.

Location:
trunk
Files:
2 added
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r167199 r167210  
     12014-04-13  Darin Adler  <darin@apple.com>
     2
     3        REGRESSION (r158617): Find on Page can get stuck in a loop when the search string occurs in an <input> in a <fieldset>
     4        https://bugs.webkit.org/show_bug.cgi?id=126322
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        * editing/text-iterator/count-matches-in-form-expected.txt: Added.
     9        * editing/text-iterator/count-matches-in-form.html: Added.
     10
     11        * fast/text/window-find.html: Tweaked the test a bit, making it a little easier to
     12        see if the test hasn't even run.
     13
    1142014-04-08  Oliver Hunt  <oliver@apple.com>
    215

  • trunk/LayoutTests/fast/text/window-find.html

    r124680 r167210  
    22<head>
    33<script>
    4 if (window.testRunner) {
    5   testRunner.dumpAsText();
    6   testRunner.waitUntilDone();
    7 }
     4
     5if (window.testRunner)
     6    testRunner.dumpAsText();
    87
    98function fail(s) {
    10   document.body.innerHTML = "FAIL: " + s;
     9    document.body.innerHTML = "FAIL: " + s;
    1110}
    1211
    1312function runTest() {
    14   // Need to force layout for window.find() to operate correctly.
    15   var forceLayout = document.body.offsetHeight;
    16   if (window.find('nonsense')) fail('found: nonsense');
    17   // https://bugs.webkit.org/show_bug.cgi?id=53654 -- failure when flipping
    18   // case sensitivity back-to-back.
    19   if (window.find('nonsense', true)) fail('found: nonsense');
    20   if (window.find('nonsense', false)) fail('found: nonsense');
    21   if (!window.find('for')) fail('not found: for');
    22   if (window.find('for')) fail('found: for');
    23   // Go backwards.
    24   if (!window.find('test', true, true, false)) fail('not found: test');
    25   if (window.find('for', true, true, false)) fail('found: for');
    26   // Backwards and case sensitivity.
    27   if (window.find('this', true, true, false)) fail('found: this');
    28   if (!window.find('This', true, true, false)) fail('not found: This');
    29   // Wrap-around forwards.
    30   if (!window.find('for', true, false, true)) fail('not found: for');
    31   if (!window.find('for', true, false, true)) fail('not found: for');
    32   // Wrap-around backwards.
    33   if (!window.find('for', true, true, true)) fail('not found: for');
    34   if (!window.find('for', true, true, true)) fail('not found: for');
    35   // Case sensitivity, forwards.
    36   if (!window.find('for', true, false, true)) fail('not found: for');
    37   if (!window.find('fOR', false, false, true)) fail('not found: for');
    38   if (!window.find('for', false, false, true)) fail('not found: for');
    39   if (!window.find('for', true, false, true)) fail('not found: for');
    40   if (window.find('FOR', true, false, true)) fail('found: FOR');
     13    if (window.find('nonsense')) fail('found: nonsense');
     14    if (window.find('nonsense', true)) fail('found: nonsense');
     15    if (window.find('nonsense', false)) fail('found: nonsense');
     16    if (!window.find('for')) fail('not found: for');
     17    if (window.find('for')) fail('found: for');
     18    // Go backwards.
     19    if (!window.find('test', true, true, false)) fail('not found: test');
     20    if (window.find('for', true, true, false)) fail('found: for');
     21    // Backwards and case sensitivity.
     22    if (window.find('this', true, true, false)) fail('found: this');
     23    if (!window.find('This', true, true, false)) fail('not found: This');
     24    // Wrap-around forwards.
     25    if (!window.find('for', true, false, true)) fail('not found: for');
     26    if (!window.find('for', true, false, true)) fail('not found: for');
     27    // Wrap-around backwards.
     28    if (!window.find('for', true, true, true)) fail('not found: for');
     29    if (!window.find('for', true, true, true)) fail('not found: for');
     30    // Case sensitivity, forwards.
     31    if (!window.find('for', true, false, true)) fail('not found: for');
     32    if (!window.find('fOR', false, false, true)) fail('not found: for');
     33    if (!window.find('for', false, false, true)) fail('not found: for');
     34    if (!window.find('for', true, false, true)) fail('not found: for');
     35    if (window.find('FOR', true, false, true)) fail('found: FOR');
    4136
    42   if (window.testRunner)
    43     testRunner.notifyDone();
     37    document.body.innerHTML = "This is a test for window.find(). SUCCESS!";
    4438}
     39
    4540</script>
    4641</head>
    4742<body onload="runTest()">
    48 This is a test for window.find(). SUCCESS!
     43This is a test for window.find(); the test has not run yet.
    4944</body>
    5045</html>

  • trunk/Source/WebCore/ChangeLog

    r167208 r167210  
     12014-04-13  Darin Adler  <darin@apple.com>
     2
     3        REGRESSION (r158617): Find on Page can get stuck in a loop when the search string occurs in an <input> in a <fieldset>
     4        https://bugs.webkit.org/show_bug.cgi?id=126322
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        * dom/Element.cpp:
     9        (WebCore::Element::canContainRangeEndPoint): Now returns false when the role of the element
     10        is "img". This is the same rule that's hard-coded in isRenderReplacedElement for the same
     11        reason. Need more test coverage to make sure this role feature works consistently.
     12
     13        * dom/Element.h: Made canContainRangeEndPoint no longer inline since it's not just a
     14        return statement any more.
     15
     16        * dom/Position.cpp:
     17        (WebCore::Position::isCandidate): Took out code that calls isRendererReplacedElement
     18        that was added in r158617; not needed now that we updated canContainRangeEndPoint.
     19
     20        * dom/Range.cpp:
     21        (WebCore::Range::firstNode): Removed code here that called isRendererReplacedElement.
     22        This was the wrong level to be adding editing logic, and there's a FIXME here to that
     23        effect, which we are now deleting. This was the change that broke Find.
     24
     25        * editing/TextIterator.cpp: Added a comment about the redundancy between the
     26        isRendererReplacedElement and editingIgnoresContent functions.
     27
     28        * html/HTMLHRElement.cpp:
     29        (WebCore::HTMLHRElement::canContainRangeEndPoint): Call through to base class instead
     30        of just returning true when we have child nodes. Lets Element::canContainRangeEndPoint
     31        do its thing.
     32        * html/HTMLHRElement.h: Ditto.
     33
     34        * html/HTMLObjectElement.cpp:
     35        (WebCore::HTMLObjectElement::canContainRangeEndPoint): Call through to base class instead
     36        of just returning true when we have fallback content. Lets Element::canContainRangeEndPoint
     37        do its thing.
     38        * html/HTMLObjectElement.h: Ditto.
     39
     40        * testing/Internals.cpp:
     41        (WebCore::Internals::countMatchesForText): Set the limit to 1000 instead of infinite.
     42
    1432014-04-12  Darin Adler  <darin@apple.com>
    244

  • trunk/Source/WebCore/accessibility/AccessibilityObject.cpp

    r167186 r167210  
    10701070    return listItem->markerTextWithSuffix();
    10711071}
    1072    
     1072
    10731073String AccessibilityObject::stringForVisiblePositionRange(const VisiblePositionRange& visiblePositionRange) const
    10741074{

  • trunk/Source/WebCore/dom/Element.cpp

    r166870 r167210  
    29402940}
    29412941
     2942bool Element::canContainRangeEndPoint() const
     2943{
     2944    return !equalIgnoringCase(fastGetAttribute(roleAttr), "img");
     2945}
     2946
    29422947} // namespace WebCore

  • trunk/Source/WebCore/dom/Element.h

    r166870 r167210  
    485485    virtual bool isSearchFieldCancelButtonElement() const { return false; }
    486486
    487     virtual bool canContainRangeEndPoint() const override { return true; }
     487    virtual bool canContainRangeEndPoint() const override;
    488488
    489489    // Used for disabled form elements; if true, prevents mouse events from being dispatched

  • trunk/Source/WebCore/dom/Position.cpp

    r165676 r167210  
    225225        return Position(m_anchorNode.get(), 0, PositionIsOffsetInAnchor);
    226226    }
     227
    227228    if (!m_anchorNode->offsetInCharacters()
    228229        && (m_anchorType == PositionIsAfterAnchor || m_anchorType == PositionIsAfterChildren || static_cast<unsigned>(m_offset) == m_anchorNode->childNodeCount())
    … …  
    936937        return false;
    937938       
    938     if (isRendererReplacedElement(renderer))
    939         return !nodeIsUserSelectNone(deprecatedNode()) && atFirstEditingPositionForNode();
    940 
    941939    if (renderer->isRenderBlockFlow()) {
    942940        RenderBlockFlow& block = toRenderBlockFlow(*renderer);

  • trunk/Source/WebCore/dom/Range.cpp

    r164964 r167210  
    15701570        return 0;
    15711571    if (m_start.container()->offsetInCharacters())
    1572         return m_start.container();
    1573     // FIXME: A renderer-based rule is completely out of place here.
    1574     // For one thing, the renderer could be out of date if the DOM was recently changed.
    1575     if (isRendererReplacedElement(m_start.container()->renderer()))
    15761572        return m_start.container();
    15771573    if (Node* child = m_start.container()->childNode(m_start.offset()))

  • trunk/Source/WebCore/editing/TextIterator.cpp

    r166507 r167210  
    243243}
    244244
     245// FIXME: editingIgnoresContent and isRendererReplacedElement try to do the same job.
     246// It's not good to have both of them.
    245247bool isRendererReplacedElement(RenderObject* renderer)
    246248{

  • trunk/Source/WebCore/html/HTMLHRElement.cpp

    r161334 r167210  
    101101}
    102102
     103bool HTMLHRElement::canContainRangeEndPoint() const
     104{
     105    return hasChildNodes() && HTMLElement::canContainRangeEndPoint();
    103106}
     107
     108}

  • trunk/Source/WebCore/html/HTMLHRElement.h

    r162180 r167210  
    3333    static PassRefPtr<HTMLHRElement> create(const QualifiedName&, Document&);
    3434
    35     virtual bool canContainRangeEndPoint() const override { return hasChildNodes(); }
    36 
    3735private:
    3836    HTMLHRElement(const QualifiedName&, Document&);
    … …  
    4038    virtual bool isPresentationAttribute(const QualifiedName&) const override;
    4139    virtual void collectStyleForPresentationAttribute(const QualifiedName&, const AtomicString&, MutableStyleProperties&) override;
     40    virtual bool canContainRangeEndPoint() const override;
    4241};
    4342

  • trunk/Source/WebCore/html/HTMLObjectElement.cpp

    r166853 r167210  
    512512}
    513513
    514 }
     514bool HTMLObjectElement::canContainRangeEndPoint() const
     515{
     516    return m_useFallbackContent && HTMLPlugInImageElement::canContainRangeEndPoint();
     517}
     518
     519}

  • trunk/Source/WebCore/html/HTMLObjectElement.h

    r166853 r167210  
    9898    virtual bool appendFormData(FormDataList&, bool) override;
    9999
    100     virtual bool canContainRangeEndPoint() const override { return useFallbackContent(); }
     100    virtual bool canContainRangeEndPoint() const override;
    101101
    102102    bool m_docNamedItem : 1;

  • trunk/Source/WebCore/testing/Internals.cpp

    r166849 r167210  
    14361436
    14371437    bool mark = markMatches == "mark";
    1438     return document->frame()->editor().countMatchesForText(text, nullptr, findOptions, std::numeric_limits<unsigned>::max(), mark, nullptr);
     1438    return document->frame()->editor().countMatchesForText(text, nullptr, findOptions, 1000, mark, nullptr);
    14391439}
    14401440
Note: See TracChangeset for help on using the changeset viewer.