Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add creator URL, creator base URL and creator origin to browsing context #26499

Merged
merged 5 commits into from May 19, 2020

Conversation

@utsavoza
Copy link
Contributor

utsavoza commented May 12, 2020

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #24944 and fix (partially) #26287
  • There are tests for these changes
@highfive
Copy link

highfive commented May 12, 2020

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/document.rs, components/script/dom/windowproxy.rs, components/script/script_thread.rs, components/script/dom/location.rs
  • @KiChjang: components/script/dom/document.rs, components/script/dom/windowproxy.rs, components/script/script_thread.rs, components/script/dom/location.rs
@jdm
Copy link
Member

jdm commented May 12, 2020

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented May 12, 2020

Trying commit 1ccb849 with merge 322cf9b...

bors-servo added a commit that referenced this pull request May 12, 2020
Add creator URL, creator base URL and creator origin to browsing context

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #24944 and fix (partially) #26287
- [x] There are tests for these changes
@bors-servo
Copy link
Contributor

bors-servo commented May 12, 2020

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented May 12, 2020

  ▶ Unexpected subtest result in /html/infrastructure/urls/terminology-0/document-base-url.html:
  └ PASS [expected FAIL] The fallback base URL of a document whose address is about:blank is the document base URL of the creator document.
  ▶ Unexpected subtest result in /html/infrastructure/urls/terminology-0/document-base-url.html:
  └ PASS [expected FAIL] about:blank with a base element.
  ▶ Unexpected subtest result in /html/webappapis/dynamic-markup-insertion/opening-the-input-stream/abort-while-navigating.window.html:
  │ TIMEOUT [expected FAIL] document.open() aborts documents that are navigating through iframe loading (XMLHttpRequest)
  └   → Test timed out
  ▶ Unexpected subtest result in /html/webappapis/dynamic-markup-insertion/opening-the-input-stream/abort-while-navigating.window.html:
  │ TIMEOUT [expected PASS] document.open() aborts documents that are navigating through iframe loading (fetch())
  └   → Test timed out
  ▶ Unexpected subtest result in /html/browsers/browsing-the-web/navigating-across-documents/010.html:
  │ FAIL [expected PASS] Link with onclick form submit to javascript url with delayed document.write and href navigation 
  │   → assert_equals: expected "href" but got "write"
  │ 
  │ onmessage<@http://web-platform.test:8000/html/browsers/browsing-the-web/navigating-across-documents/010.html:14:18
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1975:25
  └ Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:2000:35
  ▶ Unexpected subtest result in /html/semantics/document-metadata/the-base-element/base_about_blank.html:
  └ PASS [expected FAIL] base element in about:blank document should resolve against its fallback base URI
@utsavoza
Copy link
Contributor Author

utsavoza commented May 13, 2020

It seems we have a couple of unexpected test fixes and failures. After sorting them out:

  • These are valid test passes that I missed.

    ▶ Unexpected subtest result in /html/infrastructure/urls/terminology-0/document-base-url.html:
    └ PASS [expected FAIL] The fallback base URL of a document whose address is about:blank is     the document base URL of the creator document.
    
    ▶ Unexpected subtest result in /html/infrastructure/urls/terminology-0/document-base-url.html:
    └ PASS [expected FAIL] about:blank with a base element.
    
    ▶ Unexpected subtest result in /html/semantics/document-metadata/the-base-element/base_about_blank.html:
    └ PASS [expected FAIL] base element in about:blank document should resolve against its fallback base URI
    
  • Gleaning from the test, I think the reason for the timeouts might be #21937.

    ▶ Unexpected subtest result in /html/webappapis/dynamic-markup-insertion/opening-the-input-stream/abort-while-navigating.window.html:
    │ TIMEOUT [expected FAIL] document.open() aborts documents that are navigating through iframe loading (XMLHttpRequest)
    └   → Test timed out
    
    ▶ Unexpected subtest result in /html/webappapis/dynamic-markup-insertion/opening-the-input-stream/abort-while-navigating.window.html:
    │ TIMEOUT [expected PASS] document.open() aborts documents that are navigating through iframe loading (fetch())
    └   → Test timed out
    
    

    Specifically,

    // Step 8
    // TODO: https://github.com/servo/servo/issues/21937
    if self.has_browsing_context() {
    // spec says "stop document loading",
    // which is a process that does more than just abort
    self.abort();
    }

  • I am not really sure why this test failed. My best guess would be #25065, unless otherwise?

    ▶ Unexpected subtest result in /html/browsers/browsing-the-web/navigating-across-documents/010.html:
    │ FAIL [expected PASS] Link with onclick form submit to javascript url with delayed document.write and href navigation
    │   → assert_equals: expected "href" but got "write"
    │
    │ onmessage<@http://web-platform.test:8000/html/browsers/browsing-the-web/navigating-across-documents/010.html:14:18
    │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1975:25
    └ Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:2000:35
    
@utsavoza
Copy link
Contributor Author

utsavoza commented May 13, 2020

Also, after going through the follow hyperlink implementation in htmlanchorelement.rs, I think we are incorrectly parsing the subject's href attribute relative to node's document.

let url = match document.url().join(&href) {
Ok(url) => url,
Err(_) => return,
};

According to the Parse a URL algorithm, we should be parsing the href attribute value against the document's base URL. I am not sure though if that'll cause any unexpected test results as our implementation for document's fallback_base_url is still incomplete.

Copy link
Member

jdm left a comment

Looking good! This is a nice correctness improvement.

components/script/dom/document.rs Show resolved Hide resolved
components/script/dom/windowproxy.rs Outdated Show resolved Hide resolved
@jdm jdm assigned jdm and unassigned nox May 13, 2020
@utsavoza utsavoza force-pushed the utsavoza:ugo/issue-26287/10-05-2020 branch from 1ccb849 to 08c1d4b May 14, 2020
@jdm
Copy link
Member

jdm commented May 14, 2020

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

bors-servo commented May 14, 2020

Trying commit 08c1d4b with merge 3c9a09c...

bors-servo added a commit that referenced this pull request May 14, 2020
Add creator URL, creator base URL and creator origin to browsing context

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #24944 and fix (partially) #26287
- [x] There are tests for these changes
@bors-servo
Copy link
Contributor

bors-servo commented May 14, 2020

💔 Test failed - status-taskcluster

@utsavoza
Copy link
Contributor Author

utsavoza commented May 15, 2020

Some more unexpected test failures. Looking at the changes, I think adding a check for srcdoc in the document's fallback url implementation might be causing these additional test failures afaict. Will have to see what each failure entails. Let me know what you think @jdm.

@jdm
Copy link
Member

jdm commented May 15, 2020

Yes, I was expecting that outcome :)

@utsavoza
Copy link
Contributor Author

utsavoza commented May 19, 2020

  1. /FileAPI/url/sandboxed-iframe.html

    ▶ Unexpected subtest result in /FileAPI/url/sandboxed-iframe.html:
    │ TIMEOUT [expected PASS] Blob URLs can be used in <script> tags
    └   → Test timed out
    ▶ Unexpected subtest result in /FileAPI/url/sandboxed-iframe.html:
    │ FAIL [expected PASS] Blob URLs can be used in iframes, and are treated same origin
    └   → The operation is insecure.
    ▶ Unexpected subtest result in /FileAPI/url/sandboxed-iframe.html:
    │ TIMEOUT [expected PASS] Blob URL fragment is implemented.
    └   → Test timed out
    ▶ Unexpected subtest result in /FileAPI/url/sandboxed-iframe.html:
    │ FAIL [expected PASS] Blob URLs can be used in XHR
    └   → promise_test: Unhandled rejection with value: "Got unexpected error event"
    ▶ Unexpected subtest result in /FileAPI/url/sandboxed-iframe.html:
    │ FAIL [expected PASS] XHR with a fragment should succeed
    └   → promise_test: Unhandled rejection with value: "Got unexpected error event"
    ▶ Unexpected subtest result in /FileAPI/url/sandboxed-iframe.html:
    │ FAIL [expected PASS] Only exact matches should revoke URLs, using XHR
    └   → promise_test: Unhandled rejection with value: "Got unexpected error event"
    ▶ Unexpected subtest result in /FileAPI/url/sandboxed-iframe.html:
    │ FAIL [expected PASS] Revoke blob URL after open(), will fetch
    └   → assert_unreached: Got unexpected error event Reached unreachable code
    ▶ Unexpected subtest result in /FileAPI/url/sandboxed-iframe.html:
    │ FAIL [expected PASS] Blob URLs can be used in fetch
    └   → promise_test: Unhandled rejection with value: object "TypeError: Network error occurred"
    ▶ Unexpected subtest result in /FileAPI/url/sandboxed-iframe.html:
    │ FAIL [expected PASS] fetch with a fragment should succeed
    └   → promise_test: Unhandled rejection with value: object "TypeError: Network error occurred"
    ▶ Unexpected subtest result in /FileAPI/url/sandboxed-iframe.html:
    │ FAIL [expected PASS] Only exact matches should revoke URLs, using fetch
    └   → promise_test: Unhandled rejection with value: object "TypeError: Network error occurred"
    ▶ Unexpected subtest result in /FileAPI/url/sandboxed-iframe.html:
    │ FAIL [expected PASS] fetch should return Content-Type from Blob
    └   → promise_test: Unhandled rejection with value: object "TypeError: Network error occurred"
    ▶ Unexpected subtest result in /FileAPI/url/sandboxed-iframe.html:
    │ FAIL [expected PASS] Revoke blob URL after creating Request, will fetch
    └   → promise_test: Unhandled rejection with value: object "TypeError: Network error occurred"
    ▶ Unexpected subtest result in /FileAPI/url/sandboxed-iframe.html:
    │ FAIL [expected PASS] Revoke blob URL after calling fetch, fetch should succeed
    └   → promise_test: Unhandled rejection with value: object "TypeError: Network error occurred"
    ▶ Unexpected subtest result in /FileAPI/url/sandboxed-iframe.html:
    │ FAIL [expected PASS] Blob URL parses correctly
    │   → assert_true: Path must end with a valid UUID expected true got false
    │
    │ @http://web-platform.test:8000/FileAPI/url/url-format.any.js:47:14
    │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1975:25
    │ test@http://web-platform.test:8000/resources/testharness.js:535:30
    └ @http://web-platform.test:8000/FileAPI/url/url-format.any.js:39:5
    ▶ Unexpected subtest result in /FileAPI/url/sandboxed-iframe.html:
    │ FAIL [expected PASS] XHR should return Content-Type from Blob
    │   → assert_equals: expected (string) "image/png" but got (object) null
    │
    │ xhr.onloadend<@http://web-platform.test:8000/FileAPI/url/url-with-xhr.any.js:45:18
    │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1975:25
    └ Test.prototype.step_func_done/<@http://web-platform.test:8000/resources/testharness.js:2016:32
    
  2. /fetch/security/dangling-markup-mitigation.tentative.html

    ▶ Unexpected subtest result in /fetch/security/dangling-markup-mitigation.tentative.html:
    └ PASS [expected FAIL] <img id="dangling" src="/images/green-1x1.png?img=&lt;b">
    ▶ Unexpected subtest result in /fetch/security/dangling-markup-mitigation.tentative.html:
    └ PASS [expected FAIL] <img id="dangling" src="/images/green-1x1.png?img=&#10;b">
    ▶ Unexpected subtest result in /fetch/security/dangling-markup-mitigation.tentative.html:
    └ PASS [expected FAIL] <img id="dangling" src="/images/green-1x1.png?img=&amp;#10;b">
    ▶ Unexpected subtest result in /fetch/security/dangling-markup-mitigation.tentative.html:
    └ PASS [expected FAIL] <img id="dangling" src="/images/green-1x1.png?img=&amp;lt;b">
    ▶ Unexpected subtest result in /fetch/security/dangling-markup-mitigation.tentative.html:
    └ PASS [expected FAIL] <img id="dangling" src="/images/green-1x1.png?img=&amp;#10;b&amp;lt;c">
    ▶ Unexpected subtest result in /fetch/security/dangling-markup-mitigation.tentative.html:
    └ PASS [expected FAIL] \\n      <img id="dangling" src="\\n        /images/green-1x1.png?img=\\n      ">\\n
    ▶ Unexpected subtest result in /fetch/security/dangling-markup-mitigation.tentative.html:
    └ PASS [expected FAIL] \\n      <img id="dangling" src="\\n        /images/green-1x1.png?img=&amp;lt;\\n      ">\\n
    ▶ Unexpected subtest result in /fetch/security/dangling-markup-mitigation.tentative.html:
    └ PASS [expected FAIL] \\n      <img id="dangling" src="\\n        /images/green-1x1.png?img=&amp;#10;\\n      ">\\n
    ▶ Unexpected subtest result in /fetch/security/dangling-markup-mitigation.tentative.html:
    │ FAIL [expected PASS] <img id="dangling" src="/images/green-1x1.png?img=&#10;&lt;b">
    │   → assert_equals: Height expected 0 but got 1
    │ FAIL [expected PASS] <img id="dangling" src="/images/green-1x1.png?img=&lt;&#10;b">
    │   → assert_equals: Height expected 0 but got 1
    │ FAIL [expected PASS] \\n      <img id="dangling" src="/images/green-1x1.png?img=\\n        &lt;\\n        &#10;b\\n      ">\\n
    │   → assert_equals: Height expected 0 but got 1
    │
    │ assert_img_not_loaded/<@http://web-platform.test:8000/fetch/security/dangling-markup-mitigation.tentative.html:79:22
    │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1975:25
    └ Test.prototype.step_func_done/<@http://web-platform.test:8000/resources/testharness.js:2016:32
    
  3. /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_anchor_download_allow_downloads.sub.tentative.html

    ▶ OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_anchor_download_allow_downloads.sub.tentative.html
    ▶ Unexpected subtest result in /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_anchor_download_allow_downloads.sub.tentative.html:
    │ FAIL [expected TIMEOUT] <a download> triggered download in sandbox is allowed by allow-downloads.
    └   → assert_unreached: Unexpected navigation. Reached unreachable code
    
  4. /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_anchor_download_block_downloads.sub.tentative.html

    ▶ Unexpected subtest result in /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_anchor_download_block_downloads.sub.tentative.html:
    │ FAIL [expected PASS] <a download> triggered download in sandbox is blocked.
    └   → assert_unreached: Unexpected navigation. Reached unreachable code
    
  5. /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_navigation_download_allow_downloads.sub.tentative.html

    ▶ OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_navigation_download_allow_downloads.sub.tentative.html
    ▶ Unexpected subtest result in /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_navigation_download_allow_downloads.sub.tentative.html:
    │ FAIL [expected TIMEOUT] Navigation resulted download in sandbox is allowed by allow-downloads.
    └   → assert_unreached: Unexpected navigation. Reached unreachable code
    
  6. /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_navigation_download_block_downloads.sub.tentative.html

    ▶ Unexpected subtest result in /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_navigation_download_block_downloads.sub.tentative.html:
    │ FAIL [expected PASS] Navigation resulted download in sandbox is blocked.
    └   → assert_unreached: Unexpected navigation. Reached unreachable code
    
  7. /html/semantics/embedded-content/the-img-element/move-element-and-scroll.html

    ▶ Unexpected subtest result in /html/semantics/embedded-content/the-img-element/move-element-and-scroll.html:
    │ FAIL [expected PASS] Test that <img> below viewport is not loaded when moved to another document and then scrolled to
    └   → assert_unreached: The below viewport image should not load Reached unreachable code
    
  8. /html/semantics/embedded-content/the-img-element/sizes/sizes-dynamic-001.html,
    /html/semantics/embedded-content/the-img-element/sizes/sizes-dynamic-001-ref.html

    ▶ FAIL [expected PASS] /html/semantics/embedded-content/the-img-element/sizes/sizes-dynamic-001.html
    │   → /html/semantics/embedded-content/the-img-element/sizes/sizes-dynamic-001.html 7c4e7406993aa5c92015394610c118292c0593aa
    └   → /html/semantics/embedded-content/the-img-element/sizes/sizes-dynamic-001-ref.html df91e1e5441250d340a1203a86f926cde5c0426e
    
  9. /referrer-policy/generic/sandboxed-iframe-with-opaque-origin.html

    ▶ OK [expected TIMEOUT] /referrer-policy/generic/sandboxed-iframe-with-opaque-origin.html
    ▶ Unexpected subtest result in /referrer-policy/generic/sandboxed-iframe-with-opaque-origin.html:
    └ PASS [expected TIMEOUT] Sandboxed iframe with opaque origin doesn't send referrers.
    ▶ Unexpected subtest result in /referrer-policy/generic/sandboxed-iframe-with-opaque-origin.html:
    │ FAIL [expected TIMEOUT] Sandboxed iframe with tuple origin sends referrers.
    │   → assert_equals: expected (string) "http://web-platform.test:8000/referrer-policy/generic/sandboxed-iframe-with-opaque-origin.html" but got (undefined) undefined
    │
    │ testSandboxedIframe/</<@http://web-platform.test:8000/referrer-policy/generic/sandboxed-iframe-with-opaque-origin.html:19:28
    │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1975:25
    └ Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:2000:35
    
  10. /html/semantics/document-metadata/the-base-element/base_srcdoc.html

    ▶ Unexpected subtest result in /html/semantics/document-metadata/the-base-element/base_srcdoc.html:
    └ PASS [expected FAIL] base element in srcdoc document should resolve against its fallback base URI
    
  11. /css/css-values/viewport-units-after-font-load.html

    ▶ OK [expected TIMEOUT] /css/css-values/viewport-units-after-font-load.html
    ▶ Unexpected subtest result in /css/css-values/viewport-units-after-font-load.html:
    └ PASS [expected TIMEOUT] Viewport units are correctly updated after resize even if a font load has happened before
    
@utsavoza utsavoza force-pushed the utsavoza:ugo/issue-26287/10-05-2020 branch from 08c1d4b to 9b0b03a May 19, 2020
@jdm
Copy link
Member

jdm commented May 19, 2020

A bunch of the new failures that I looked at like https://searchfox.org/mozilla-central/source/testing/web-platform/tests/html/semantics/embedded-content/the-img-element/move-element-and-scroll.html make sense since they rely on features we don't support yet. Looks good!

@jdm
Copy link
Member

jdm commented May 19, 2020

@bors-servo
Copy link
Contributor

bors-servo commented May 19, 2020

📌 Commit 9b0b03a has been approved by jdm

@utsavoza
Copy link
Contributor Author

utsavoza commented May 19, 2020

@jdm, I looked at the test failures but I am not sure what actually caused them in:

  1. /FileAPI/url/sandboxed-iframe.html
  2. /fetch/security/dangling-markup-mitigation.tentative.html
  3. /html/semantics/embedded-content/the-img-element/sizes/sizes-dynamic-001.html

Also, I noticed there are many open issues that partly concern the test failures, but I am also not sure if we need to create any new issues to track any of these?

@jdm
Copy link
Member

jdm commented May 19, 2020

sandboxed-iframe.html and dangling-markup-mitigation.html started resolving URLs correctly inside the srcdoc iframe for the first time, so the test failures represent tests that didn't run at all previously.

I believe the failure in sizes-dynamic-001.html is caused again by the URLs now resolving correctly, and I think we don't implement intrinsic sizes for responsive images so the test now loads images that don't match the reference file.

@utsavoza
Copy link
Contributor Author

utsavoza commented May 19, 2020

I see. Let me know if we require any other changes.

@bors-servo
Copy link
Contributor

bors-servo commented May 19, 2020

Testing commit 9b0b03a with merge cab9104...

@bors-servo
Copy link
Contributor

bors-servo commented May 19, 2020

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing cab9104 to master...

@bors-servo bors-servo merged commit cab9104 into servo:master May 19, 2020
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.