Changeset 231456 in webkit


Ignore:
Timestamp:
May 7, 2018 2:42:29 PM (6 years ago)
Author:
Chris Dumez
Message:

Stop using an iframe's id as fallback if its name attribute is not set
​https://bugs.webkit.org/show_bug.cgi?id=11388

Reviewed by Geoff Garen.

Source/WebCore:

WebKit had logic to use an iframe's id as fallback name when its name
content attribute is not set. This behavior was not standard and did not
match other browsers:

Gecko / Trident never behaved this way. Blink was aligned with us until
they started to match the specification in:

This WebKit quirk was causing some Web-compatibility issues because it
would affect the behavior of Window's name property getter when trying
to look up an iframe by id. Because of Window's named property getter
behavior [1], we would return the frame's contentWindow instead of the
iframe element itself.

[1] ​https://html.spec.whatwg.org/multipage/window-object.html#named-access-on-the-window-object

Test: fast/dom/Window/named-getter-frame-id.html

  • html/HTMLFrameElementBase.cpp:

(WebCore::HTMLFrameElementBase::openURL):
(WebCore::HTMLFrameElementBase::parseAttribute):
(WebCore::HTMLFrameElementBase::didFinishInsertingNode):

  • html/HTMLFrameElementBase.h:

LayoutTests:

  • fast/dom/Window/named-getter-frame-id-expected.txt: Added.
  • fast/dom/Window/named-getter-frame-id.html: Added.

Add layout test coverage.

  • fast/dom/Geolocation/srcdoc-getCurrentPosition-expected.txt:
  • fast/dom/Geolocation/srcdoc-watchPosition-expected.txt:
  • fast/dom/HTMLAnchorElement/anchor-in-noscroll-iframe-crash.html:
  • fast/dom/Window/window-special-properties-expected.txt:
  • fast/frames/iframe-no-name-expected.txt:
  • fast/frames/iframe-no-name.html:
  • fast/layers/prevent-hit-test-during-layout.html:
  • fast/xmlhttprequest/xmlhttprequest-no-file-access-expected.txt:
  • http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin.html:
  • http/tests/security/contentSecurityPolicy/iframe-blank-url-programmatically-add-external-script-expected.txt:
  • http/tests/security/cross-origin-reified-window-property-access.html:
  • http/tests/storageAccess/request-and-grant-storage-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-non-recent-user-interaction-and-try-access-from-right-frame-expected.txt:
  • http/tests/storageAccess/request-and-grant-storage-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-non-recent-user-interaction-but-try-access-from-wrong-frame-expected.txt:
  • http/tests/webrtc/filtering-ice-candidate-same-origin-frame.html:
  • http/wpt/beacon/keepalive-after-navigation-expected.txt:
  • http/wpt/cache-storage/cache-remove-twice.html:

Update some layout tests that relied on our old (non-standard) behavior.

Location:
trunk
Files:
2 added
25 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r231453 r231456  
     12018-05-07  Chris Dumez  <cdumez@apple.com>
     2
     3        Stop using an iframe's id as fallback if its name attribute is not set
     4        https://bugs.webkit.org/show_bug.cgi?id=11388
     5
     6        Reviewed by Geoff Garen.
     7
     8        * fast/dom/Window/named-getter-frame-id-expected.txt: Added.
     9        * fast/dom/Window/named-getter-frame-id.html: Added.
     10        Add layout test coverage.
     11
     12        * fast/dom/Geolocation/srcdoc-getCurrentPosition-expected.txt:
     13        * fast/dom/Geolocation/srcdoc-watchPosition-expected.txt:
     14        * fast/dom/HTMLAnchorElement/anchor-in-noscroll-iframe-crash.html:
     15        * fast/dom/Window/window-special-properties-expected.txt:
     16        * fast/frames/iframe-no-name-expected.txt:
     17        * fast/frames/iframe-no-name.html:
     18        * fast/layers/prevent-hit-test-during-layout.html:
     19        * fast/xmlhttprequest/xmlhttprequest-no-file-access-expected.txt:
     20        * http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin.html:
     21        * http/tests/security/contentSecurityPolicy/iframe-blank-url-programmatically-add-external-script-expected.txt:
     22        * http/tests/security/cross-origin-reified-window-property-access.html:
     23        * http/tests/storageAccess/request-and-grant-storage-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-non-recent-user-interaction-and-try-access-from-right-frame-expected.txt:
     24        * http/tests/storageAccess/request-and-grant-storage-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-non-recent-user-interaction-but-try-access-from-wrong-frame-expected.txt:
     25        * http/tests/webrtc/filtering-ice-candidate-same-origin-frame.html:
     26        * http/wpt/beacon/keepalive-after-navigation-expected.txt:
     27        * http/wpt/cache-storage/cache-remove-twice.html:
     28        Update some layout tests that relied on our old (non-standard) behavior.
     29
    1302018-05-07  Youenn Fablet  <youenn@apple.com>
    231

  • trunk/LayoutTests/fast/dom/Geolocation/srcdoc-getCurrentPosition-expected.txt

    r231367 r231456  
    44
    55--------
    6 Frame: 'frame'
     6Frame: '<!--frame1-->'
    77--------
    88FAIL should have invoked error callback, but invoked success callback.

  • trunk/LayoutTests/fast/dom/Geolocation/srcdoc-watchPosition-expected.txt

    r231367 r231456  
    44
    55--------
    6 Frame: 'frame'
     6Frame: '<!--frame1-->'
    77--------
    88FAIL should have invoked error callback, but invoked success callback.

  • trunk/LayoutTests/fast/dom/HTMLAnchorElement/anchor-in-noscroll-iframe-crash.html

    r231367 r231456  
    1515
    1616    function setupTopLevel() {
    17         var scrollTarget = window.frames['target'].document.getElementById('might_scroll');
     17        var scrollTarget = iframeTarget.contentDocument.getElementById('might_scroll');
    1818
    19         window.frames['target'].window.registerAction(function () {
     19        iframeTarget.contentWindow.registerAction(function () {
    2020            iframeTarget.remove();
    2121            setTimeout(finish, 0);
    2222        });
    2323
    24         window.frames['target'].window.run();
     24        iframeTarget.contentWindow.run();
    2525    }
    2626</script>   

  • trunk/LayoutTests/fast/dom/Window/window-special-properties-expected.txt

    r231367 r231456  
    4343Iframe by name (unique): single WINDOW
    4444Iframe by name (multiple): single WINDOW
    45 Iframe by id (unique): single WINDOW
    46 Iframe by id (multiple): single WINDOW
     45Iframe by id (unique): single IFRAME(id)
     46Iframe by id (multiple): collection(2) IFRAME(id) IFRAME(id)
    4747Iframe by id/name mixed: single WINDOW
    4848
    … …  
    5454Span by id/name mixed: collection(2) SPAN(id) SPAN(id)
    5555
    56 Mixed by id: single WINDOW
     56Mixed by id: collection(7) IMG(id) FORM(id) APPLET(id) EMBED(id) OBJECT(id) IFRAME(id) SPAN(id)
    5757Mixed by name: single WINDOW
    5858Mixed by id (no iframe): collection(6) IMG(id) FORM(id) APPLET(id) EMBED(id) OBJECT(id) SPAN(id)

  • trunk/LayoutTests/fast/frames/iframe-no-name-expected.txt

    r231367 r231456  
    44
    55
    6 PASS frames[0].name is "id"
     6PASS frames[0].name is ""
    77PASS frames[1].name is "name"
    88PASS frames[2].name is "name"

  • trunk/LayoutTests/fast/frames/iframe-no-name.html

    r231367 r231456  
    1212<script>
    1313description("Checks that the id of an iframe does not set the contentWindow's name if the iframe's name is not set.");
    14 shouldBeEqualToString("frames[0].name", "id");
     14shouldBeEqualToString("frames[0].name", "");
    1515shouldBeEqualToString("frames[1].name", "name");
    1616shouldBeEqualToString("frames[2].name", "name");

  • trunk/LayoutTests/fast/layers/prevent-hit-test-during-layout.html

    r231367 r231456  
    3434   
    3535    function runTest() {
    36         fixedDiv = window.frames['fixedFrame'].document.getElementById('fixedDiv');
     36        fixedDiv = fixedFrame.contentDocument.getElementById('fixedDiv');
    3737        target = document.getElementById('target');
    3838

  • trunk/LayoutTests/fast/xmlhttprequest/xmlhttprequest-no-file-access-expected.txt

    r231450 r231456  
    1717
    1818--------
    19 Frame: 'f'
     19Frame: '<!--frame2-->'
    2020--------
    2121Successful write into iframe

  • trunk/LayoutTests/http/tests/loading/basic-auth-load-URL-with-consecutive-slashes-expected.txt

    r231367 r231456  
    11main frame - didStartProvisionalLoadForFrame
    22main frame - didCommitLoadForFrame
    3 frame "frame" - didStartProvisionalLoadForFrame
     3frame "<!--frame1-->" - didStartProvisionalLoadForFrame
    44main frame - didFinishDocumentLoadForFrame
    55http://127.0.0.1:8000/loading/resources/basic-auth-testing.php?username=webkit&password=rocks - didReceiveAuthenticationChallenge - Responding with webkit:rocks
    6 frame "frame" - didCommitLoadForFrame
    7 frame "frame" - didFinishDocumentLoadForFrame
    8 frame "frame" - willPerformClientRedirectToURL: http://127.0.0.1:8000/a//b/non-existent-file.html
    9 frame "frame" - didHandleOnloadEventsForFrame
     6frame "<!--frame1-->" - didCommitLoadForFrame
     7frame "<!--frame1-->" - didFinishDocumentLoadForFrame
     8frame "<!--frame1-->" - willPerformClientRedirectToURL: http://127.0.0.1:8000/a//b/non-existent-file.html
     9frame "<!--frame1-->" - didHandleOnloadEventsForFrame
    1010main frame - didHandleOnloadEventsForFrame
    11 frame "frame" - didFinishLoadForFrame
     11frame "<!--frame1-->" - didFinishLoadForFrame
    1212main frame - didFinishLoadForFrame
    13 frame "frame" - didStartProvisionalLoadForFrame
    14 frame "frame" - didCancelClientRedirectForFrame
    15 frame "frame" - didCommitLoadForFrame
    16 frame "frame" - didReceiveTitle: 404 Not Found
    17 frame "frame" - didFinishDocumentLoadForFrame
    18 frame "frame" - didFailLoadWithError
     13frame "<!--frame1-->" - didStartProvisionalLoadForFrame
     14frame "<!--frame1-->" - didCancelClientRedirectForFrame
     15frame "<!--frame1-->" - didCommitLoadForFrame
     16frame "<!--frame1-->" - didReceiveTitle: 404 Not Found
     17frame "<!--frame1-->" - didFinishDocumentLoadForFrame
     18frame "<!--frame1-->" - didFailLoadWithError
    1919PASS did not cause assertion failure.

  • trunk/LayoutTests/http/tests/navigation/image-load-in-subframe-unload-handler-expected.txt

    r231450 r231456  
    1 frame "<!--frame1-->" - has 1 onunload handler(s)
     1frame "<!--frame2-->" - has 1 onunload handler(s)
    22This test triggers an unload handler that starts an image load in a different frame (and deletes both frames), but ensures the main frame is not destroyed. We pass if we don't crash.

  • trunk/LayoutTests/http/tests/quicklook/csp-header-ignored-expected.txt

    r231367 r231456  
    55
    66--------
    7 Frame: 'frame'
     7Frame: '<!--frame1-->'
    88--------
    99PASS

  • trunk/LayoutTests/http/tests/security/clipboard/copy-paste-html-cross-origin-iframe-across-origin.html

    r231367 r231456  
    3434    getSelection().removeAllRanges();
    3535    setTimeout(() => {
    36         destinationFrame.postMessage({type: 'paste'}, '*');
     36        destinationFrame.contentWindow.postMessage({type: 'paste'}, '*');
    3737    }, 0);
    3838}

  • trunk/LayoutTests/http/tests/security/contentSecurityPolicy/iframe-blank-url-programmatically-add-external-script-expected.txt

    r231367 r231456  
    33
    44--------
    5 Frame: 'frame'
     5Frame: '<!--frame1-->'
    66--------
    77

  • trunk/LayoutTests/http/tests/security/cross-origin-reified-window-property-access.html

    r231367 r231456  
    2727function runTest()
    2828{
    29     crossOriginWindow = crossOriginFrame.window;
    30     sameOriginWindow = sameOriginFrame.window;
     29    crossOriginWindow = crossOriginFrame.contentWindow;
     30    sameOriginWindow = sameOriginFrame.contentWindow;
    3131
    3232    shouldThrowOrReturnUndefined('crossOriginWindow.document');

  • trunk/LayoutTests/http/tests/storageAccess/request-and-grant-storage-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-non-recent-user-interaction-and-try-access-from-right-frame-expected.txt

    r231450 r231456  
    1111
    1212--------
    13 Frame: 'TheIframeThatRequestsStorageAccess'
     13Frame: '<!--frame1-->'
    1414--------
    1515After the top frame navigates the sub frame, the sub frame should no longer have access to first-party cookies.
    … …  
    1919
    2020--------
    21 Frame: '<!--frame1-->'
     21Frame: '<!--frame2-->'
    2222--------
    2323Should receive first-party cookie.
    … …  
    2727
    2828--------
    29 Frame: '<!--frame2-->'
     29Frame: '<!--frame3-->'
    3030--------
    3131Should not receive cookies.
    … …  
    3535
    3636--------
    37 Frame: '<!--frame3-->'
     37Frame: '<!--frame4-->'
    3838--------
    3939
    … …  
    4141
    4242--------
    43 Frame: '<!--frame4-->'
     43Frame: '<!--frame5-->'
    4444--------
    4545Should receive partitioned cookie.

  • trunk/LayoutTests/http/tests/storageAccess/request-and-grant-storage-access-cross-origin-sandboxed-iframe-from-prevalent-domain-with-non-recent-user-interaction-but-try-access-from-wrong-frame-expected.txt

    r231450 r231456  
    1111
    1212--------
    13 Frame: 'theIframe'
     13Frame: '<!--frame1-->'
    1414--------
    1515
    1616
    1717--------
    18 Frame: '<!--frame1-->'
     18Frame: '<!--frame2-->'
    1919--------
    2020Should receive first-party cookie.
    … …  
    2424
    2525--------
    26 Frame: '<!--frame2-->'
     26Frame: '<!--frame3-->'
    2727--------
    2828Should not receive cookies.
    … …  
    3232
    3333--------
    34 Frame: '<!--frame3-->'
     34Frame: '<!--frame4-->'
    3535--------
    3636
    3737
    38 
    39 --------
    40 Frame: '<!--frame4-->'
    41 --------
    42 Should receive partitioned cookie.
    43 Did not receive cookie named 'firstPartyCookie'.
    44 Received cookie named 'partitionedCookie'.
    45 Client-side document.cookie: partitionedCookie=value
    4638
    4739--------
    … …  
    5244Received cookie named 'partitionedCookie'.
    5345Client-side document.cookie: partitionedCookie=value
     46
     47--------
     48Frame: '<!--frame6-->'
     49--------
     50Should receive partitioned cookie.
     51Did not receive cookie named 'firstPartyCookie'.
     52Received cookie named 'partitionedCookie'.
     53Client-side document.cookie: partitionedCookie=value

  • trunk/LayoutTests/http/tests/webrtc/filtering-ice-candidate-same-origin-frame.html

    r231367 r231456  
    1616    if (event.data === "getUserMedia done") {
    1717        didGetUserMedia = true;
    18         frame1.postMessage("check filtering", "*");
     18        frame1.contentWindow.postMessage("check filtering", "*");
    1919        return;
    2020    }

  • trunk/LayoutTests/http/wpt/beacon/keepalive-after-navigation-expected.txt

    r231367 r231456  
    1 frame "testFrame" - has 1 onunload handler(s)
     1frame "<!--frame1-->" - has 1 onunload handler(s)
    22
    33PASS Test that beacon sent from unload event handler is properly received

  • trunk/LayoutTests/http/wpt/cache-storage/cache-remove-twice.html

    r231367 r231456  
    2525        return new Promise((resolve, reject) => {
    2626            window.addEventListener("message", test.step_func((event) => {
    27                 return Promise.all([self.caches.open(cacheName), cacheFrame.window.caches.open(cacheName) ]).then(() => {
    28                     return Promise.all([self.caches.delete(cacheName), cacheFrame.window.caches.delete(cacheName)]);
     27                return Promise.all([self.caches.open(cacheName), cacheFrame.contentWindow.caches.open(cacheName) ]).then(() => {
     28                    return Promise.all([self.caches.delete(cacheName), cacheFrame.contentWindow.caches.delete(cacheName)]);
    2929                }).then(resolve, reject);
    3030            }));

  • trunk/LayoutTests/platform/ios/http/tests/quicklook/csp-header-ignored-expected.txt

    r231367 r231456  
    44
    55--------
    6 Frame: 'frame'
     6Frame: '<!--frame1-->'
    77--------
    88PASS

  • trunk/LayoutTests/platform/wk2/http/tests/loading/basic-auth-load-URL-with-consecutive-slashes-expected.txt

    r231367 r231456  
    22main frame - didCommitLoadForFrame
    33main frame - didFinishDocumentLoadForFrame
    4 frame "frame" - didStartProvisionalLoadForFrame
     4frame "<!--frame1-->" - didStartProvisionalLoadForFrame
    55127.0.0.1:8000 - didReceiveAuthenticationChallenge - Responding with webkit:rocks
    6 frame "frame" - didCommitLoadForFrame
    7 frame "frame" - didFinishDocumentLoadForFrame
    8 frame "frame" - willPerformClientRedirectToURL: http://127.0.0.1:8000/a//b/non-existent-file.html
    9 frame "frame" - didHandleOnloadEventsForFrame
     6frame "<!--frame1-->" - didCommitLoadForFrame
     7frame "<!--frame1-->" - didFinishDocumentLoadForFrame
     8frame "<!--frame1-->" - willPerformClientRedirectToURL: http://127.0.0.1:8000/a//b/non-existent-file.html
     9frame "<!--frame1-->" - didHandleOnloadEventsForFrame
    1010main frame - didHandleOnloadEventsForFrame
    11 frame "frame" - didFinishLoadForFrame
     11frame "<!--frame1-->" - didFinishLoadForFrame
    1212main frame - didFinishLoadForFrame
    13 frame "frame" - didStartProvisionalLoadForFrame
    14 frame "frame" - didCancelClientRedirectForFrame
    15 frame "frame" - didCommitLoadForFrame
    16 frame "frame" - didReceiveTitle: 404 Not Found
    17 frame "frame" - didFinishDocumentLoadForFrame
    18 frame "frame" - didFailLoadWithError
     13frame "<!--frame1-->" - didStartProvisionalLoadForFrame
     14frame "<!--frame1-->" - didCancelClientRedirectForFrame
     15frame "<!--frame1-->" - didCommitLoadForFrame
     16frame "<!--frame1-->" - didReceiveTitle: 404 Not Found
     17frame "<!--frame1-->" - didFinishDocumentLoadForFrame
     18frame "<!--frame1-->" - didFailLoadWithError
    1919PASS did not cause assertion failure.

  • trunk/Source/WebCore/ChangeLog

    r231450 r231456  
     12018-05-07  Chris Dumez  <cdumez@apple.com>
     2
     3        Stop using an iframe's id as fallback if its name attribute is not set
     4        https://bugs.webkit.org/show_bug.cgi?id=11388
     5
     6        Reviewed by Geoff Garen.
     7
     8        WebKit had logic to use an iframe's id as fallback name when its name
     9        content attribute is not set. This behavior was not standard and did not
     10        match other browsers:
     11        - https://html.spec.whatwg.org/#attr-iframe-name
     12
     13        Gecko / Trident never behaved this way. Blink was aligned with us until
     14        they started to match the specification in:
     15        - https://bugs.chromium.org/p/chromium/issues/detail?id=347169
     16
     17        This WebKit quirk was causing some Web-compatibility issues because it
     18        would affect the behavior of Window's name property getter when trying
     19        to look up an iframe by id. Because of Window's named property getter
     20        behavior [1], we would return the frame's contentWindow instead of the
     21        iframe element itself.
     22
     23        [1] https://html.spec.whatwg.org/multipage/window-object.html#named-access-on-the-window-object
     24
     25        Test: fast/dom/Window/named-getter-frame-id.html
     26
     27        * html/HTMLFrameElementBase.cpp:
     28        (WebCore::HTMLFrameElementBase::openURL):
     29        (WebCore::HTMLFrameElementBase::parseAttribute):
     30        (WebCore::HTMLFrameElementBase::didFinishInsertingNode):
     31        * html/HTMLFrameElementBase.h:
     32
    1332018-05-07  Chris Dumez  <cdumez@apple.com>
    234

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

    r231367 r231456  
    9797        return;
    9898
    99     parentFrame->loader().subframeLoader().requestFrame(*this, m_URL, m_frameName, lockHistory, lockBackForwardList);
     99    parentFrame->loader().subframeLoader().requestFrame(*this, m_URL, getNameAttribute(), lockHistory, lockBackForwardList);
    100100}
    101101
    … …  
    106106    else if (name == srcAttr && !hasAttributeWithoutSynchronization(srcdocAttr))
    107107        setLocation(stripLeadingAndTrailingHTMLSpaces(value));
    108     else if (name == idAttr) {
    109         HTMLFrameOwnerElement::parseAttribute(name, value);
    110         // Falling back to using the 'id' attribute is not standard but some content relies on this behavior.
    111         if (!hasAttributeWithoutSynchronization(nameAttr))
    112             m_frameName = value;
    113     } else if (name == nameAttr) {
    114         m_frameName = value;
    115         // FIXME: If we are already attached, this doesn't actually change the frame's name.
    116         // FIXME: If we are already attached, this doesn't check for frame name
    117         // conflicts and generate a unique frame name.
    118     } else if (name == marginwidthAttr) {
     108    else if (name == marginwidthAttr) {
    119109        m_marginWidth = value.toInt();
    120110        // FIXME: If we are already attached, this has no effect.
    … …  
    133123}
    134124
    135 void HTMLFrameElementBase::setNameAndOpenURL()
    136 {
    137     m_frameName = getNameAttribute();
    138     // Falling back to using the 'id' attribute is not standard but some content relies on this behavior.
    139     if (m_frameName.isNull())
    140         m_frameName = getIdAttribute();
    141     openURL();
    142 }
    143 
    144125Node::InsertedIntoAncestorResult HTMLFrameElementBase::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
    145126{
    … …  
    164145    if (!renderer())
    165146        invalidateStyleAndRenderersForSubtree();
    166     setNameAndOpenURL();
     147    openURL();
    167148}
    168149

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

    r231367 r231456  
    7171    bool isFrameElementBase() const final { return true; }
    7272
    73     void setNameAndOpenURL();
    7473    void openURL(LockHistory = LockHistory::Yes, LockBackForwardList = LockBackForwardList::Yes);
    7574
    7675    AtomicString m_URL;
    77     AtomicString m_frameName;
    7876
    7977    ScrollbarMode m_scrolling;
Note: See TracChangeset for help on using the changeset viewer.