Changeset 122537 in webkit


Ignore:
Timestamp:
Jul 12, 2012 7:12:21 PM (12 years ago)
Author:
eric@webkit.org
Message:

Incorrect behaviour calling Range setStart or setEnd with boundary in different document
​https://bugs.webkit.org/show_bug.cgi?id=42517

Reviewed by Ojan Vafai.

Source/WebCore:

Added a new static inline "checkForDifferentRootContainer" to share some code
and made setStart/setEnd do the right thing in the x-document case. I removed
the bogus checks in set*After/set*Before functions, and since they just call
through to setStart/setEnd, they also now do the right thing.

Test: fast/dom/Range/set-wrong-document-err.html

  • dom/Range.cpp:

(WebCore::checkForDifferentRootContainer):
(WebCore):
(WebCore::Range::setStart):
(WebCore::Range::setEnd):
(WebCore::Range::setStartAfter):
(WebCore::Range::setEndBefore):
(WebCore::Range::setEndAfter):
(WebCore::Range::setStartBefore):

LayoutTests:

Add a new test to cover this changed behavior, and correct a FIXME in an old test
which was documenting our incorrect behavior.

  • fast/dom/Range/set-wrong-document-err-expected.txt: Added.
  • fast/dom/Range/set-wrong-document-err.html: Added.
  • fast/dom/move-nodes-across-documents.html:
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r122532 r122537  
     12012-07-12  Eric Seidel  <eric@webkit.org>
     2
     3        Incorrect behaviour calling Range setStart or setEnd with boundary in different document
     4        https://bugs.webkit.org/show_bug.cgi?id=42517
     5
     6        Reviewed by Ojan Vafai.
     7
     8        Add a new test to cover this changed behavior, and correct a FIXME in an old test
     9        which was documenting our incorrect behavior.
     10
     11        * fast/dom/Range/set-wrong-document-err-expected.txt: Added.
     12        * fast/dom/Range/set-wrong-document-err.html: Added.
     13        * fast/dom/move-nodes-across-documents.html:
     14
    1152012-07-12  Konrad Piascik  <kpiascik@rim.com>
    216

  • trunk/LayoutTests/fast/dom/move-nodes-across-documents.html

    r120792 r122537  
    182182    }, 'HIERARCHY_REQUEST_ERR');
    183183
    184     // FIXME: This doesn't match http://www.w3.org/TR/DOM-Level-2-Traversal-Range/ranges.html#Level-2-Range-Changing.
    185     // Gecko implements it correctly. When setting a boundary of the range in a different
     184    // When setting a boundary of the range in a different
    186185    // document, the call should succeed and the range should be collapsed.
    187186    runTest(function() {
    188187        rangeInIframe().setStart(elementInCurrentDocument('setStart'), 0);
    189     }, 'WRONG_DOCUMENT_ERR');
     188    });
    190189    runTest(function() {
    191190        rangeInIframe().setEnd(elementInCurrentDocument('setEnd'), 0);
    192     }, 'WRONG_DOCUMENT_ERR');
     191    });
    193192    runTest(function() {
    194193        rangeInIframe().setStartBefore(elementInCurrentDocument('setStartBefore'), 0);
    195     }, 'WRONG_DOCUMENT_ERR');
     194    });
    196195    runTest(function() {
    197196        rangeInIframe().setStartAfter(elementInCurrentDocument('setStartAfter'), 0);
    198     }, 'WRONG_DOCUMENT_ERR');
     197    });
    199198    runTest(function() {
    200199        rangeInIframe().setEndBefore(elementInCurrentDocument('setEndBefore'), 0);
    201     }, 'WRONG_DOCUMENT_ERR');
     200    });
    202201    runTest(function() {
    203202        rangeInIframe().setEndAfter(elementInCurrentDocument('setEndAfter'), 0);
    204     }, 'WRONG_DOCUMENT_ERR');
     203    });
    205204
    206205    // FIXME: isPointInRange isn't specced anywhere, but Gecko doesn't throw an exception here.

  • trunk/Source/WebCore/ChangeLog

    r122535 r122537  
     12012-07-12  Eric Seidel  <eric@webkit.org>
     2
     3        Incorrect behaviour calling Range setStart or setEnd with boundary in different document
     4        https://bugs.webkit.org/show_bug.cgi?id=42517
     5
     6        Reviewed by Ojan Vafai.
     7
     8        Added a new static inline "checkForDifferentRootContainer" to share some code
     9        and made setStart/setEnd do the right thing in the x-document case.  I removed
     10        the bogus checks in set*After/set*Before functions, and since they just call
     11        through to setStart/setEnd, they also now do the right thing.
     12
     13        Test: fast/dom/Range/set-wrong-document-err.html
     14
     15        * dom/Range.cpp:
     16        (WebCore::checkForDifferentRootContainer):
     17        (WebCore):
     18        (WebCore::Range::setStart):
     19        (WebCore::Range::setEnd):
     20        (WebCore::Range::setStartAfter):
     21        (WebCore::Range::setEndBefore):
     22        (WebCore::Range::setEndAfter):
     23        (WebCore::Range::setStartBefore):
     24
    1252012-07-12  Erik Arvidsson  <arv@chromium.org>
    226

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

    r119690 r122537  
    197197}
    198198
     199static inline bool checkForDifferentRootContainer(const RangeBoundaryPoint& start, const RangeBoundaryPoint& end)
     200{
     201    Node* endRootContainer = end.container();
     202    while (endRootContainer->parentNode())
     203        endRootContainer = endRootContainer->parentNode();
     204    Node* startRootContainer = start.container();
     205    while (startRootContainer->parentNode())
     206        startRootContainer = startRootContainer->parentNode();
     207
     208    return startRootContainer != endRootContainer || (Range::compareBoundaryPoints(start, end, ASSERT_NO_EXCEPTION) > 0);
     209}
     210
    199211void Range::setStart(PassRefPtr<Node> refNode, int offset, ExceptionCode& ec)
    200212{
    … …  
    209221    }
    210222
     223    bool didMoveDocument = false;
    211224    if (refNode->document() != m_ownerDocument) {
    212         ec = WRONG_DOCUMENT_ERR;
    213         return;
     225        setDocument(refNode->document());
     226        didMoveDocument = true;
    214227    }
    215228
    … …  
    221234    m_start.set(refNode, offset, childNode);
    222235
    223     // check if different root container
    224     Node* endRootContainer = m_end.container();
    225     while (endRootContainer->parentNode())
    226         endRootContainer = endRootContainer->parentNode();
    227     Node* startRootContainer = m_start.container();
    228     while (startRootContainer->parentNode())
    229         startRootContainer = startRootContainer->parentNode();
    230     if (startRootContainer != endRootContainer)
     236    if (didMoveDocument || checkForDifferentRootContainer(m_start, m_end))
    231237        collapse(true, ec);
    232     // check if new start after end
    233     else if (compareBoundaryPoints(m_start, m_end, ec) > 0) {
    234         ASSERT(!ec);
    235         collapse(true, ec);
    236     }
    237238}
    238239
    … …  
    249250    }
    250251
     252    bool didMoveDocument = false;
    251253    if (refNode->document() != m_ownerDocument) {
    252         ec = WRONG_DOCUMENT_ERR;
    253         return;
     254        setDocument(refNode->document());
     255        didMoveDocument = true;
    254256    }
    255257
    … …  
    261263    m_end.set(refNode, offset, childNode);
    262264
    263     // check if different root container
    264     Node* endRootContainer = m_end.container();
    265     while (endRootContainer->parentNode())
    266         endRootContainer = endRootContainer->parentNode();
    267     Node* startRootContainer = m_start.container();
    268     while (startRootContainer->parentNode())
    269         startRootContainer = startRootContainer->parentNode();
    270     if (startRootContainer != endRootContainer)
     265    if (didMoveDocument || checkForDifferentRootContainer(m_start, m_end))
    271266        collapse(false, ec);
    272     // check if new end before start
    273     if (compareBoundaryPoints(m_start, m_end, ec) > 0) {
    274         ASSERT(!ec);
    275         collapse(false, ec);
    276     }
    277267}
    278268
    … …  
    12541244    }
    12551245
    1256     if (refNode->document() != m_ownerDocument) {
    1257         ec = WRONG_DOCUMENT_ERR;
    1258         return;
    1259     }
    1260 
    12611246    ec = 0;
    12621247    checkNodeBA(refNode, ec);
    … …  
    12791264    }
    12801265
    1281     if (refNode->document() != m_ownerDocument) {
    1282         ec = WRONG_DOCUMENT_ERR;
    1283         return;
    1284     }
    1285 
    12861266    ec = 0;
    12871267    checkNodeBA(refNode, ec);
    … …  
    13041284    }
    13051285
    1306     if (refNode->document() != m_ownerDocument) {
    1307         ec = WRONG_DOCUMENT_ERR;
    1308         return;
    1309     }
    1310 
    13111286    ec = 0;
    13121287    checkNodeBA(refNode, ec);
    … …  
    13151290
    13161291    setEnd(refNode->parentNode(), refNode->nodeIndex() + 1, ec);
    1317 
    13181292}
    13191293
    … …  
    15301504    }
    15311505
    1532     if (refNode->document() != m_ownerDocument) {
    1533         ec = WRONG_DOCUMENT_ERR;
    1534         return;
    1535     }
    1536 
    15371506    ec = 0;
    15381507    checkNodeBA(refNode, ec);
Note: See TracChangeset for help on using the changeset viewer.