Changeset 209628 in webkit


Ignore:
Timestamp:
Dec 9, 2016 2:06:29 PM (7 years ago)
Author:
rniwa@webkit.org
Message:

document.webkitFullscreenElement leaks elements inside a shadow tree
https://bugs.webkit.org/show_bug.cgi?id=158471

Reviewed by Chris Dumez.

Source/WebCore:

Fixed the bug by calling the newly added ancestorElementInThisScope in webkitCurrentFullScreenElementForBindings
and webkitFullscreenElementForBinding.

The specification (https://fullscreen.spec.whatwg.org/#dom-document-fullscreenelement) uses "the result of
retargeting fullscreen element" and returns null if the result is not in the same tree as the context object.

This is equivalent to the algorithm implemented by ancestorElementInThisScope. Observe that the retargeting
algorithm (https://dom.spec.whatwg.org/#retarget) finds the lowest common tree scope of the retargetee and
the context object. There are two cases to consider.

  1. The context object's tree scope is the lowest common tree scope: In this case, an ancestor shadow host or

the retargetee itself is in this tree scope. It's sufficient traverse every shadow host to find the one that
resides in the same tree scope as the context object. This is precisely what ancestorElementInThisScope does.

  1. The context object's tree scope is not the lowest common tree scope: In this case, the context object is

inside a shadow tree whose ancestor shadow host is in the lowest common tree scope. In this case, retargeting
algorithm finds a node which is not in the same tree as the context object. Thus, the result is null.
ancestorElementInThisScope traveres ancestor shadow hosts and returns null if no shadow host's tree scope
matches that of the context object's tree scope. Thus, it would return null in this case as desired.

Also renamed TreeScope::focusedElement to focusedElementInScope for clarity since Document which inherits
from TreeScope also has a distinct member function named focusedElement called by TreeScope::focusedElement,
and used ancestorElementInThisScope since it uses the same algorithm.

Tests: fast/shadow-dom/activeElement-for-focused-element-in-another-shadow.html

fast/shadow-dom/blur-on-shadow-host-with-focused-shadow-content.html
fast/shadow-dom/fullscreen-in-shadow-fullscreenElement.html
fast/shadow-dom/fullscreen-in-shadow-webkitCurrentFullScreenElement.html
fast/shadow-dom/fullscreen-in-slot-fullscreenElement.html
fast/shadow-dom/fullscreen-in-slot-webkitCurrentFullScreenElement.html

  • dom/Document.cpp:

(WebCore::Document::removeFocusedNodeOfSubtree):
(WebCore::Document::activeElement):

  • dom/Document.h:

(WebCore::Document::webkitCurrentFullScreenElementForBindings): Added.
(WebCore::Document::webkitFullscreenElementForBindings): Added.

  • dom/Document.idl:
  • dom/Element.cpp:

(WebCore::Element::blur):

  • dom/ShadowRoot.h:

(WebCore::ShadowRoot::activeElement):

  • dom/TreeScope.cpp:

(WebCore::TreeScope::ancestorNodeInThisScope): Renamed from ancestorInThisScope for clarity.
(WebCore::TreeScope::ancestorElementInThisScope):
(WebCore::TreeScope::focusedElementInScope): Renamed from focusedElement to disambiguate it from Document's
focusedElement.

  • dom/TreeScope.h:
  • editing/VisibleSelection.cpp:

(WebCore::adjustPositionForEnd):
(WebCore::adjustPositionForStart):

  • editing/htmlediting.cpp:

(WebCore::comparePositions):
(WebCore::firstEditablePositionAfterPositionInRoot):
(WebCore::lastEditablePositionBeforePositionInRoot):

  • page/DOMSelection.cpp:

(WebCore::selectionShadowAncestor):
(WebCore::DOMSelection::shadowAdjustedNode):
(WebCore::DOMSelection::shadowAdjustedOffset):

  • rendering/HitTestResult.cpp:

(WebCore::HitTestResult::addNodeToRectBasedTestResult): Added a FIXME here since this is clearly wrong for
shadow trees created by author scripts.

Source/WebKit/mac:

Use the API for bindings to avoid exposing nodes inside a shadow tree.

  • DOM/DOMDocument.mm:

(-[DOMDocument webkitCurrentFullScreenElement]):
(-[DOMDocument webkitFullscreenElement]):

LayoutTests:

Added tests for calling webkitFullscreenElement and webkitCurrentFullScreenElement on a fullscreened element
to make sure they return the shadow host instead.

Also added two unrelated test cases for temporal regressions I introduced while working on this patch.

Skip the fullscreen tests on iOS WK2 since eventSender doesn't work there.

  • fast/shadow-dom/activeElement-for-focused-element-in-another-shadow-expected.txt: Added.
  • fast/shadow-dom/activeElement-for-focused-element-in-another-shadow.html: Added.
  • fast/shadow-dom/blur-on-shadow-host-with-focused-shadow-content-expected.txt: Added.
  • fast/shadow-dom/blur-on-shadow-host-with-focused-shadow-content.html: Added.
  • fast/shadow-dom/fullscreen-in-shadow-fullscreenElement-expected.txt: Added.
  • fast/shadow-dom/fullscreen-in-shadow-fullscreenElement.html: Added.
  • fast/shadow-dom/fullscreen-in-shadow-webkitCurrentFullScreenElement-expected.txt: Added.
  • fast/shadow-dom/fullscreen-in-shadow-webkitCurrentFullScreenElement.html: Added.
  • fast/shadow-dom/fullscreen-in-slot-fullscreenElement-expected.txt: Added.
  • fast/shadow-dom/fullscreen-in-slot-fullscreenElement.html: Added.
  • fast/shadow-dom/fullscreen-in-slot-webkitCurrentFullScreenElement-expected.txt: Added.
  • fast/shadow-dom/fullscreen-in-slot-webkitCurrentFullScreenElement.html: Added.
  • platform/ios-simulator-wk2/TestExpectations:
Location:
trunk
Files:
12 added
16 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r209620 r209628  
     12016-12-09  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        document.webkitFullscreenElement leaks elements inside a shadow tree
     4        https://bugs.webkit.org/show_bug.cgi?id=158471
     5
     6        Reviewed by Chris Dumez.
     7
     8        Added tests for calling webkitFullscreenElement and webkitCurrentFullScreenElement on a fullscreened element
     9        to make sure they return the shadow host instead.
     10
     11        Also added two unrelated test cases for temporal regressions I introduced while working on this patch.
     12
     13        Skip the fullscreen tests on iOS WK2 since eventSender doesn't work there.
     14
     15        * fast/shadow-dom/activeElement-for-focused-element-in-another-shadow-expected.txt: Added.
     16        * fast/shadow-dom/activeElement-for-focused-element-in-another-shadow.html: Added.
     17        * fast/shadow-dom/blur-on-shadow-host-with-focused-shadow-content-expected.txt: Added.
     18        * fast/shadow-dom/blur-on-shadow-host-with-focused-shadow-content.html: Added.
     19        * fast/shadow-dom/fullscreen-in-shadow-fullscreenElement-expected.txt: Added.
     20        * fast/shadow-dom/fullscreen-in-shadow-fullscreenElement.html: Added.
     21        * fast/shadow-dom/fullscreen-in-shadow-webkitCurrentFullScreenElement-expected.txt: Added.
     22        * fast/shadow-dom/fullscreen-in-shadow-webkitCurrentFullScreenElement.html: Added.
     23        * fast/shadow-dom/fullscreen-in-slot-fullscreenElement-expected.txt: Added.
     24        * fast/shadow-dom/fullscreen-in-slot-fullscreenElement.html: Added.
     25        * fast/shadow-dom/fullscreen-in-slot-webkitCurrentFullScreenElement-expected.txt: Added.
     26        * fast/shadow-dom/fullscreen-in-slot-webkitCurrentFullScreenElement.html: Added.
     27        * platform/ios-simulator-wk2/TestExpectations:
     28
    1292016-12-09  Chris Dumez  <cdumez@apple.com>
    230
  • trunk/LayoutTests/platform/ios-simulator-wk2/TestExpectations

    r209448 r209628  
    17791779fast/dom/Window/post-message-user-action.html [ Skip ]
    17801780fast/shadow-dom/click-text-inside-linked-slot.html [ Skip ]
     1781fast/shadow-dom/fullscreen-in-shadow-fullscreenElement.html
     1782fast/shadow-dom/fullscreen-in-shadow-webkitCurrentFullScreenElement.html
     1783fast/shadow-dom/fullscreen-in-slot-fullscreenElement.html
     1784fast/shadow-dom/fullscreen-in-slot-webkitCurrentFullScreenElement.html
    17811785
    17821786# No touch events
  • trunk/Source/WebCore/ChangeLog

    r209627 r209628  
     12016-12-09  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        document.webkitFullscreenElement leaks elements inside a shadow tree
     4        https://bugs.webkit.org/show_bug.cgi?id=158471
     5
     6        Reviewed by Chris Dumez.
     7
     8        Fixed the bug by calling the newly added ancestorElementInThisScope in webkitCurrentFullScreenElementForBindings
     9        and webkitFullscreenElementForBinding.
     10
     11        The specification (https://fullscreen.spec.whatwg.org/#dom-document-fullscreenelement) uses "the result of
     12        retargeting fullscreen element" and returns null if the result is not in the same tree as the context object.
     13
     14        This is equivalent to the algorithm implemented by ancestorElementInThisScope. Observe that the retargeting
     15        algorithm (https://dom.spec.whatwg.org/#retarget) finds the lowest common tree scope of the retargetee and
     16        the context object. There are two cases to consider.
     17
     18        1. The context object's tree scope is the lowest common tree scope: In this case, an ancestor shadow host or
     19        the retargetee itself is in this tree scope. It's sufficient traverse every shadow host to find the one that
     20        resides in the same tree scope as the context object. This is precisely what ancestorElementInThisScope does.
     21
     22        2. The context object's tree scope is not the lowest common tree scope: In this case, the context object is
     23        inside a shadow tree whose ancestor shadow host is in the lowest common tree scope. In this case, retargeting
     24        algorithm finds a node which is not in the same tree as the context object. Thus, the result is null.
     25        ancestorElementInThisScope traveres ancestor shadow hosts and returns null if no shadow host's tree scope
     26        matches that of the context object's tree scope. Thus, it would return null in this case as desired.
     27
     28        Also renamed TreeScope::focusedElement to focusedElementInScope for clarity since Document which inherits
     29        from TreeScope also has a distinct member function named focusedElement called by TreeScope::focusedElement,
     30        and used ancestorElementInThisScope since it uses the same algorithm.
     31
     32        Tests: fast/shadow-dom/activeElement-for-focused-element-in-another-shadow.html
     33               fast/shadow-dom/blur-on-shadow-host-with-focused-shadow-content.html
     34               fast/shadow-dom/fullscreen-in-shadow-fullscreenElement.html
     35               fast/shadow-dom/fullscreen-in-shadow-webkitCurrentFullScreenElement.html
     36               fast/shadow-dom/fullscreen-in-slot-fullscreenElement.html
     37               fast/shadow-dom/fullscreen-in-slot-webkitCurrentFullScreenElement.html
     38
     39        * dom/Document.cpp:
     40        (WebCore::Document::removeFocusedNodeOfSubtree):
     41        (WebCore::Document::activeElement):
     42        * dom/Document.h:
     43        (WebCore::Document::webkitCurrentFullScreenElementForBindings): Added.
     44        (WebCore::Document::webkitFullscreenElementForBindings): Added.
     45        * dom/Document.idl:
     46        * dom/Element.cpp:
     47        (WebCore::Element::blur):
     48        * dom/ShadowRoot.h:
     49        (WebCore::ShadowRoot::activeElement):
     50        * dom/TreeScope.cpp:
     51        (WebCore::TreeScope::ancestorNodeInThisScope): Renamed from ancestorInThisScope for clarity.
     52        (WebCore::TreeScope::ancestorElementInThisScope):
     53        (WebCore::TreeScope::focusedElementInScope): Renamed from focusedElement to disambiguate it from Document's
     54        focusedElement.
     55        * dom/TreeScope.h:
     56        * editing/VisibleSelection.cpp:
     57        (WebCore::adjustPositionForEnd):
     58        (WebCore::adjustPositionForStart):
     59        * editing/htmlediting.cpp:
     60        (WebCore::comparePositions):
     61        (WebCore::firstEditablePositionAfterPositionInRoot):
     62        (WebCore::lastEditablePositionBeforePositionInRoot):
     63        * page/DOMSelection.cpp:
     64        (WebCore::selectionShadowAncestor):
     65        (WebCore::DOMSelection::shadowAdjustedNode):
     66        (WebCore::DOMSelection::shadowAdjustedOffset):
     67        * rendering/HitTestResult.cpp:
     68        (WebCore::HitTestResult::addNodeToRectBasedTestResult): Added a FIXME here since this is clearly wrong for
     69        shadow trees created by author scripts.
     70
    1712016-12-09  Geoffrey Garen  <ggaren@apple.com>
    272
  • trunk/Source/WebCore/dom/Document.cpp

    r209608 r209628  
    35413541        return;
    35423542
    3543     Element* focusedElement = node.treeScope().focusedElement();
     3543    Element* focusedElement = node.treeScope().focusedElementInScope();
    35443544    if (!focusedElement)
    35453545        return;
     
    67726772Element* Document::activeElement()
    67736773{
    6774     if (Element* element = treeScope().focusedElement())
     6774    if (Element* element = treeScope().focusedElementInScope())
    67756775        return element;
    67766776    return bodyOrFrameset();
  • trunk/Source/WebCore/dom/Document.h

    r209403 r209628  
    10891089    bool webkitFullScreenKeyboardInputAllowed() const { return m_fullScreenElement.get() && m_areKeysEnabledInFullScreen; }
    10901090    Element* webkitCurrentFullScreenElement() const { return m_fullScreenElement.get(); }
    1091    
     1091    Element* webkitCurrentFullScreenElementForBindings() const { return ancestorElementInThisScope(webkitCurrentFullScreenElement()); }
     1092
    10921093    enum FullScreenCheckType {
    10931094        EnforceIFrameAllowFullScreenRequirement,
     
    11161117    WEBCORE_EXPORT bool webkitFullscreenEnabled() const;
    11171118    Element* webkitFullscreenElement() const { return !m_fullScreenElementStack.isEmpty() ? m_fullScreenElementStack.last().get() : nullptr; }
     1119    Element* webkitFullscreenElementForBindings() const { return ancestorElementInThisScope(webkitFullscreenElement()); }
    11181120    WEBCORE_EXPORT void webkitExitFullscreen();
    11191121#endif
  • trunk/Source/WebCore/dom/Document.idl

    r209514 r209628  
    144144    readonly attribute boolean webkitIsFullScreen;
    145145    readonly attribute boolean webkitFullScreenKeyboardInputAllowed;
    146     readonly attribute Element webkitCurrentFullScreenElement;
     146    [ImplementedAs=webkitCurrentFullScreenElementForBindings] readonly attribute Element webkitCurrentFullScreenElement;
    147147    void webkitCancelFullScreen();
    148148
    149149    // W3C version
    150150    readonly attribute boolean webkitFullscreenEnabled;
    151     readonly attribute Element? webkitFullscreenElement;
     151    [ImplementedAs=webkitFullscreenElementForBindings] readonly attribute Element? webkitFullscreenElement;
    152152    void webkitExitFullscreen();
    153153#endif
  • trunk/Source/WebCore/dom/Element.cpp

    r209446 r209628  
    24392439{
    24402440    cancelFocusAppearanceUpdate();
    2441     if (treeScope().focusedElement() == this) {
     2441    if (treeScope().focusedElementInScope() == this) {
    24422442        if (Frame* frame = document().frame())
    24432443            frame->page()->focusController().setFocusedElement(0, frame);
  • trunk/Source/WebCore/dom/ShadowRoot.h

    r208967 r209628  
    111111inline Element* ShadowRoot::activeElement() const
    112112{
    113     return treeScope().focusedElement();
     113    return treeScope().focusedElementInScope();
    114114}
    115115
  • trunk/Source/WebCore/dom/TreeScope.cpp

    r208828 r209628  
    196196}
    197197
    198 Node* TreeScope::ancestorInThisScope(Node* node) const
     198Node* TreeScope::ancestorNodeInThisScope(Node* node) const
    199199{
    200200    for (; node; node = node->shadowHost()) {
     
    202202            return node;
    203203        if (!node->isInShadowTree())
     204            return nullptr;
     205    }
     206    return nullptr;
     207}
     208
     209Element* TreeScope::ancestorElementInThisScope(Element* element) const
     210{
     211    for (; element; element = element->shadowHost()) {
     212        if (&element->treeScope() == this)
     213            return element;
     214        if (!element->isInShadowTree())
    204215            return nullptr;
    205216    }
     
    365376}
    366377
    367 Element* TreeScope::focusedElement()
     378Element* TreeScope::focusedElementInScope()
    368379{
    369380    Document& document = m_rootNode.document();
     
    372383    if (!element && document.page())
    373384        element = focusedFrameOwnerElement(document.page()->focusController().focusedFrame(), document.frame());
    374     if (!element)
    375         return nullptr;
    376     TreeScope* treeScope = &element->treeScope();
    377     RELEASE_ASSERT(&document == &treeScope->documentScope());
    378     while (treeScope != this && treeScope != &document) {
    379         auto& rootNode = treeScope->rootNode();
    380         if (is<ShadowRoot>(rootNode))
    381             element = downcast<ShadowRoot>(rootNode).host();
    382         else
    383             return nullptr;
    384         treeScope = &element->treeScope();
    385     }
    386     if (this != treeScope)
    387         return nullptr;
    388     return element;
     385
     386    return ancestorElementInThisScope(element);
    389387}
    390388
  • trunk/Source/WebCore/dom/TreeScope.h

    r208828 r209628  
    5252    void setParentTreeScope(TreeScope&);
    5353
    54     Element* focusedElement();
     54    Element* focusedElementInScope();
    5555    WEBCORE_EXPORT Element* getElementById(const AtomicString&) const;
    5656    WEBCORE_EXPORT Element* getElementById(const String&) const;
     
    7373    Node& retargetToScope(Node&) const;
    7474
    75     Node* ancestorInThisScope(Node*) const;
     75    Node* ancestorNodeInThisScope(Node*) const;
     76    WEBCORE_EXPORT Element* ancestorElementInThisScope(Element*) const;
    7677
    7778    void addImageMap(HTMLMapElement&);
  • trunk/Source/WebCore/editing/VisibleSelection.cpp

    r208479 r209628  
    474474    ASSERT(&currentPosition.containerNode()->treeScope() != &treeScope);
    475475
    476     if (Node* ancestor = treeScope.ancestorInThisScope(currentPosition.containerNode())) {
     476    if (Node* ancestor = treeScope.ancestorNodeInThisScope(currentPosition.containerNode())) {
    477477        if (ancestor->contains(startContainerNode))
    478478            return positionAfterNode(ancestor);
     
    492492    ASSERT(&currentPosition.containerNode()->treeScope() != &treeScope);
    493493   
    494     if (Node* ancestor = treeScope.ancestorInThisScope(currentPosition.containerNode())) {
     494    if (Node* ancestor = treeScope.ancestorNodeInThisScope(currentPosition.containerNode())) {
    495495        if (ancestor->contains(endContainerNode))
    496496            return positionBeforeNode(ancestor);
  • trunk/Source/WebCore/editing/htmlediting.cpp

    r209436 r209628  
    8484        return 0;
    8585
    86     Node* nodeA = commonScope->ancestorInThisScope(a.containerNode());
     86    Node* nodeA = commonScope->ancestorNodeInThisScope(a.containerNode());
    8787    ASSERT(nodeA);
    8888    bool hasDescendentA = nodeA != a.containerNode();
    8989    int offsetA = hasDescendentA ? 0 : a.computeOffsetInContainerNode();
    9090
    91     Node* nodeB = commonScope->ancestorInThisScope(b.containerNode());
     91    Node* nodeB = commonScope->ancestorNodeInThisScope(b.containerNode());
    9292    ASSERT(nodeB);
    9393    bool hasDescendentB = nodeB != b.containerNode();
     
    293293
    294294    if (&position.deprecatedNode()->treeScope() != &highestRoot->treeScope()) {
    295         auto* shadowAncestor = highestRoot->treeScope().ancestorInThisScope(position.deprecatedNode());
     295        auto* shadowAncestor = highestRoot->treeScope().ancestorNodeInThisScope(position.deprecatedNode());
    296296        if (!shadowAncestor)
    297297            return { };
     
    321321
    322322    if (&position.deprecatedNode()->treeScope() != &highestRoot->treeScope()) {
    323         auto* shadowAncestor = highestRoot->treeScope().ancestorInThisScope(position.deprecatedNode());
     323        auto* shadowAncestor = highestRoot->treeScope().ancestorNodeInThisScope(position.deprecatedNode());
    324324        if (!shadowAncestor)
    325325            return { };
  • trunk/Source/WebCore/page/DOMSelection.cpp

    r208479 r209628  
    5050        return nullptr;
    5151    // FIXME: Unclear on why this needs to be the possibly null frame.document() instead of the never null node->document().
    52     return frame.document()->ancestorInThisScope(node);
     52    return frame.document()->ancestorNodeInThisScope(node);
    5353}
    5454
     
    437437
    438438    auto* containerNode = position.containerNode();
    439     auto* adjustedNode = m_frame->document()->ancestorInThisScope(containerNode);
     439    auto* adjustedNode = m_frame->document()->ancestorNodeInThisScope(containerNode);
    440440    if (!adjustedNode)
    441441        return nullptr;
     
    453453
    454454    auto* containerNode = position.containerNode();
    455     auto* adjustedNode = m_frame->document()->ancestorInThisScope(containerNode);
     455    auto* adjustedNode = m_frame->document()->ancestorNodeInThisScope(containerNode);
    456456    if (!adjustedNode)
    457457        return 0;
  • trunk/Source/WebCore/rendering/HitTestResult.cpp

    r208630 r209628  
    669669        return true;
    670670
     671    // FIXME: This moves out of a author shadow tree.
    671672    if (request.disallowsUserAgentShadowContent())
    672         node = node->document().ancestorInThisScope(node);
     673        node = node->document().ancestorNodeInThisScope(node);
    673674
    674675    mutableRectBasedTestResult().add(node);
     
    689690        return true;
    690691
     692    // FIXME: This moves out of a author shadow tree.
    691693    if (request.disallowsUserAgentShadowContent())
    692         node = node->document().ancestorInThisScope(node);
     694        node = node->document().ancestorNodeInThisScope(node);
    693695
    694696    mutableRectBasedTestResult().add(node);
  • trunk/Source/WebKit/mac/ChangeLog

    r209626 r209628  
     12016-12-09  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        document.webkitFullscreenElement leaks elements inside a shadow tree
     4        https://bugs.webkit.org/show_bug.cgi?id=158471
     5
     6        Reviewed by Chris Dumez.
     7
     8        Use the API for bindings to avoid exposing nodes inside a shadow tree.
     9
     10        * DOM/DOMDocument.mm:
     11        (-[DOMDocument webkitCurrentFullScreenElement]):
     12        (-[DOMDocument webkitFullscreenElement]):
     13
    1142016-12-09  Beth Dakin  <bdakin@apple.com>
    215
  • trunk/Source/WebKit/mac/DOM/DOMDocument.mm

    r208659 r209628  
    362362{
    363363    WebCore::JSMainThreadNullState state;
    364     return kit(WTF::getPtr(IMPL->webkitCurrentFullScreenElement()));
     364    return kit(WTF::getPtr(IMPL->webkitCurrentFullScreenElementForBindings()));
    365365}
    366366
     
    374374{
    375375    WebCore::JSMainThreadNullState state;
    376     return kit(WTF::getPtr(IMPL->webkitFullscreenElement()));
     376    return kit(WTF::getPtr(IMPL->webkitFullscreenElementForBindings()));
    377377}
    378378
Note: See TracChangeset for help on using the changeset viewer.