Skip to content

Don't run scripts in documents that don't have a browsing context#35848

Closed
simonwuelker wants to merge 1 commit intoservo:mainfrom
simonwuelker:no-script-unless-bc
Closed

Don't run scripts in documents that don't have a browsing context#35848
simonwuelker wants to merge 1 commit intoservo:mainfrom
simonwuelker:no-script-unless-bc

Conversation

@simonwuelker
Copy link
Contributor

@simonwuelker simonwuelker commented Mar 7, 2025

For confirmation that this is correct, refer to the note under Step 3 of https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#dom-domparser-parsefromstring.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes (covered by wpt)

@simonwuelker
Don't run scripts in document that don't have a browsing context
d6dfb8e 
For confirmation that this is correct, refer to the note under
Step 3 of https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#dom-domparser-parsefromstring.

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>
@simonwuelker simonwuelker changed the title Don't run scripts in document that don't have a browsing context Don't run scripts in documents that don't have a browsing context Mar 7, 2025
@simonwuelker simonwuelker added the T-linux-wpt Do a try run of the WPT label Mar 7, 2025
@github-actions github-actions bot removed the T-linux-wpt Do a try run of the WPT label Mar 7, 2025
@github-actions
Copy link

github-actions bot commented Mar 7, 2025

🔨 Triggering try run (#13724581632) for Linux (WPT)

@github-actions
Copy link

github-actions bot commented Mar 7, 2025

Test results for linux-wpt-layout-2020 from try job (#13724581632):

Flaky unexpected result (22)
  • OK /encoding/legacy-mb-japanese/shift_jis/sjis-encode-form.html?1-1000
    • FAIL [expected PASS] subtest: U+80 � %80

      assert_equals: expected "%80" but got ""

    • FAIL [expected PASS] subtest: U+A5 ¥ %5C

      str is undefined

    • FAIL [expected PASS] subtest: U+A7 § %81%98

      str is undefined

    • FAIL [expected PASS] subtest: U+A8 ¨ %81%4E

      str is undefined

    • FAIL [expected PASS] subtest: U+B0 ° %81%8B

      str is undefined

    • FAIL [expected PASS] subtest: U+B1 ± %81%7D

      str is undefined

    • FAIL [expected PASS] subtest: U+B4 ´ %81%4C

      str is undefined

    • FAIL [expected PASS] subtest: U+B6 ¶ %81%F7

      str is undefined

    • FAIL [expected PASS] subtest: U+D7 × %81%7E

      str is undefined

    • FAIL [expected PASS] subtest: U+F7 ÷ %81%80

      str is undefined

    • And 390 more unexpected results...
  • OK /encoding/legacy-mb-korean/euc-kr/euckr-encode-form-ksc5601.html?1-1000
    • FAIL [expected PASS] subtest: U+2533 ┳ %A6%B3

      assert_equals: expected "%A6%B3" but got ""

    • FAIL [expected PASS] subtest: U+2534 ┴ %A6%AA

      str is undefined

    • FAIL [expected PASS] subtest: U+2535 ┵ %A6%D5

      str is undefined

    • FAIL [expected PASS] subtest: U+2536 ┶ %A6%D6

      str is undefined

    • FAIL [expected PASS] subtest: U+2537 ┷ %A6%BA

      str is undefined

    • FAIL [expected PASS] subtest: U+2538 ┸ %A6%BF

      str is undefined

    • FAIL [expected PASS] subtest: U+2539 ┹ %A6%D7

      str is undefined

    • FAIL [expected PASS] subtest: U+253A ┺ %A6%D8

      str is undefined

    • FAIL [expected PASS] subtest: U+253B ┻ %A6%B5

      str is undefined

    • FAIL [expected PASS] subtest: U+253C ┼ %A6%AB

      str is undefined

    • And 590 more unexpected results...
  • OK [expected CRASH] /fetch/api/response/response-stream-with-broken-then.any.html (#35419)
  • OK [expected ERROR] /fetch/fetch-later/quota/same-origin-iframe/oversized-payload.tentative.https.window.html (#35210)
  • OK /fetch/metadata/generated/css-font-face.sub.tentative.html (#34624)
    • PASS [expected FAIL] subtest: sec-fetch-storage-access - Not sent to non-trustworthy same-site destination
  • TIMEOUT /fetch/metadata/generated/css-images.sub.tentative.html (#29047)
    • TIMEOUT [expected PASS] subtest: background-image sec-fetch-site - Not sent to non-trustworthy cross-site destination

      Test timed out

  • OK /html/browsers/browsing-the-web/navigating-across-documents/008.html (#24456)
    • PASS [expected FAIL] subtest: Link with onclick form submit to javascript url and href navigation
  • OK /html/browsers/browsing-the-web/navigating-across-documents/empty-iframe-load-event.html (#29066)
    • FAIL [expected PASS] subtest: Check execution order from nested timeout

      assert_equals: Expected nested setTimeout to run second expected true but got false

    • FAIL [expected PASS] subtest: Check execution order on load handler

      assert_equals: Expected onload to run first expected false but got true

  • OK /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/iframe-nosrc.html (#34819)
    • PASS [expected FAIL] subtest: form submission
  • OK /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/a-click.html (#28697)
    • PASS [expected FAIL] subtest: aElement.click() before the load event must NOT replace
  • OK /html/browsers/history/the-history-interface/traverse_the_history_3.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • OK /html/browsers/history/the-history-interface/traverse_the_history_5.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • PASS [expected FAIL] /html/canvas/element/manual/drawing-text-to-the-canvas/canvas.2d.disconnected-font-size-math.html (#30063)
  • TIMEOUT [expected ERROR] /html/canvas/element/manual/imagebitmap/createImageBitmap-invalid-args.html (#32745)
    • TIMEOUT [expected FAIL] subtest: createImageBitmap with an HTMLVideoElement from a data URL source and sw set to 0

      Test timed out

    • NOTRUN [expected FAIL] subtest: createImageBitmap with an HTMLVideoElement from a data URL source and sh set to 0
    • NOTRUN [expected FAIL] subtest: createImageBitmap with an HTMLVideoElement from a data URL source and oversized (unallocatable) crop region
    • NOTRUN [expected FAIL] subtest: createImageBitmap with an HTMLVideoElement from a data URL source and a value of 0 int resizeWidth
    • NOTRUN [expected FAIL] subtest: createImageBitmap with an HTMLVideoElement from a data URL source and a value of 0 in resizeHeight
    • NOTRUN [expected FAIL] subtest: createImageBitmap with an HTMLVideoElement from a data URL source and a value between 0 and 1 in resizeWidth
    • NOTRUN [expected FAIL] subtest: createImageBitmap with an HTMLVideoElement from a data URL source and a value between 0 and 1 in resizeHeight
    • NOTRUN [expected FAIL] subtest: createImageBitmap with a bitmap HTMLImageElement source and sw set to 0
    • NOTRUN [expected FAIL] subtest: createImageBitmap with a bitmap HTMLImageElement source and sh set to 0
    • NOTRUN [expected FAIL] subtest: createImageBitmap with a bitmap HTMLImageElement source and oversized (unallocatable) crop region
    • And 12 more unexpected results...
  • OK [expected ERROR] /html/canvas/element/manual/imagebitmap/createImageBitmap-premultiplyAlpha.html
  • ERROR [expected TIMEOUT] /html/canvas/element/manual/imagebitmap/createImageBitmap-transfer.html (#34119)
  • OK /html/interaction/focus/the-autofocus-attribute/document-with-fragment-valid.html (#28259)
    • PASS [expected FAIL] subtest: Autofocus elements in top-level browsing context's documents with URL fragments should be skipped.
  • OK /html/semantics/embedded-content/the-iframe-element/iframe-loading-lazy-reload-location-reload.html (#32595)
    • FAIL [expected PASS] subtest: Reloading iframe loading='lazy' before it is loaded: location.reload

      uncaught exception: Error: assert_equals: expected "http://web-platform.test:8000/html/semantics/embedded-content/the-iframe-element/support/blank.htm?src" but got "about:blank"

  • OK /html/semantics/forms/form-submission-0/multipart-formdata.window.html (#28725)
    • PASS [expected FAIL] subtest: multipart/form-data: 0x00 in value (normal form)
  • TIMEOUT [expected OK] /html/semantics/forms/form-submission-0/reparent-form-during-planned-navigation-task.html (#29724)
    • TIMEOUT [expected PASS] subtest: reparent-form-during-planned-navigation-task

      Test timed out

  • OK /html/semantics/forms/form-submission-0/urlencoded2.window.html (#28687)
    • PASS [expected FAIL] subtest: application/x-www-form-urlencoded: 0x00 in value (normal form)
  • OK /html/webappapis/user-prompts/print-during-unload.html
    • FAIL [expected PASS] subtest: print() during unload

      assert_array_equals: expected property 1 to be "destination" but got "error: window.print is not a function" (expected array ["start", "destination"] got ["start", "error: window.print is not a function"])

Stable unexpected results that are known to be intermittent (15)
  • OK /FileAPI/url/url-with-fetch.any.worker.html (#21517)
    • PASS [expected FAIL] subtest: Revoke blob URL after calling fetch, fetch should succeed
  • PASS [expected FAIL] /css/css-tables/table-cell-overflow-auto-scrolled.html (#35011)
  • OK /css/css-values/cap-invalidation.html (#32757)
    • FAIL [expected PASS] subtest: CSS Values and Units Test: cap invalidation

      uncaught exception: Error: assert_not_equals: expect the capital height of Ahem and sans-serif to be different got disallowed value 371.3333333333333

  • OK /fetch/metadata/generated/css-font-face.https.sub.tentative.html (#32732)
    • PASS [expected FAIL] subtest: sec-fetch-dest
  • OK /html/browsers/browsing-the-web/navigating-across-documents/navigation-unload-same-origin.window.html (#29049)
    • PASS [expected FAIL] subtest: Same-origin navigation started from unload handler must be ignored
  • TIMEOUT [expected OK] /html/browsers/browsing-the-web/navigating-across-documents/replace-before-load/form-submit-button-click.html (#28765)
    • TIMEOUT [expected FAIL] subtest: Replace before load, triggered by submitButton.click()

      Test timed out

  • OK /html/browsers/history/the-history-interface/traverse_the_history_4.html (#21383)
    • PASS [expected FAIL] subtest: Multiple history traversals, last would be aborted
  • PASS [expected FAIL] /html/canvas/element/manual/text/canvas.2d.disconnected.html (#30063)
  • CRASH [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-1.html (#22647)
  • TIMEOUT [expected OK] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_escaping-2.html (#22667)
    • TIMEOUT [expected FAIL] subtest: Check that popups from a sandboxed iframe escape the sandbox if allow-popups-to-escape-sandbox is used

      Test timed out

  • TIMEOUT /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-2.html (#22154)
    • NOTRUN [expected FAIL] subtest: Check that popups from a sandboxed iframe do not escape the sandbox
  • OK [expected TIMEOUT] /html/semantics/embedded-content/the-iframe-element/iframe_sandbox_popups_nonescaping-3.html (#24066)
  • OK /html/semantics/forms/form-submission-0/form-submit-iframe-then-location-navigate.html (#29634)
    • FAIL [expected PASS] subtest: Verifies that location navigations take precedence when following form submissions.

      myframe.contentDocument.location is null

  • OK [expected TIMEOUT] /mixed-content/gen/top.http-rp/opt-in/audio-tag.https.html (#35744)
  • OK /navigation-timing/test-navigation-type-reload.html (#33334)
    • FAIL [expected PASS] subtest: Reload domComplete &gt; Original domComplete

      assert_true: Reload domComplete &gt; Original domComplete expected true got false

    • FAIL [expected PASS] subtest: Reload loadEventEnd &gt; Original loadEventEnd

      assert_true: Reload loadEventEnd &gt; Original loadEventEnd expected true got false

    • FAIL [expected PASS] subtest: Reload loadEventStart &gt; Original loadEventStart

      assert_true: Reload loadEventStart &gt; Original loadEventStart expected true got false

Stable unexpected results (2)
  • OK /domparsing/DOMParser-parseFromString-html.html
    • PASS [expected FAIL] subtest: must be parsed with scripting disabled, so noscript works
  • OK /html/syntax/parsing-html-fragments/tokenizer-modes-001.html
    • FAIL [expected PASS] subtest: &lt;/noscript&gt; should not break out of noscript.

      assert_equals: expected 0 but got 1

@github-actions
Copy link

github-actions bot commented Mar 7, 2025

⚠️ Try run (#13724581632) failed.

@simonwuelker
Copy link
Contributor Author

simonwuelker commented Mar 9, 2025

I'm not sure whether that is a bug in our parsing code or whether this change is wrong as a whole. I'll close this until I figure out where exactly in the spec it says that scripts outside browsing contexts should not be run.

@jdm
Copy link
Member

jdm commented Mar 9, 2025

https://html.spec.whatwg.org/#concept-n-noscript

@simonwuelker
Copy link
Contributor Author

simonwuelker commented Mar 9, 2025 •
edited
Loading

Odd, https://html.spec.whatwg.org/multipage/parsing.html#concept-frag-parse-context clearly says that when the context element is a noscript element and the scripting flag is disabled, we should start the tokenizer in the Data state. So of course we parse the div tag and it ends up in the dom (which, according to the test, should not happen)

@simonwuelker
Copy link
Contributor Author

simonwuelker commented Mar 9, 2025 •
edited
Loading

Aha! Ladybird uses the scripting flag of the context element when parsing html fragments 1, we mistakenly use the scripting flag of the parser 2. That makes sense, because the noscript element doesn't belong to the parser!

I'll mark this test case as a failure for now, as it seems clear to me that the bug is in html5ever.

Footnotes

  1. https://github.com/LadybirdBrowser/ladybird/blob/a37315da875e4a12130d08c5e4e0912a897c8cdb/Libraries/LibWeb/HTML/Parser/HTMLParser.cpp#L4583-L4585 ↩

  2. https://github.com/servo/html5ever/blob/6b3601add6f18389092759b3069c5facce6a7912/html5ever/src/tree_builder/mod.rs#L252-L258 ↩

This was referenced Mar 9, 2025
Closed
Merged
@jdm
Copy link
Member

jdm commented Mar 9, 2025 •
edited
Loading

#35478 may be relevant.

@simonwuelker
Copy link
Contributor Author

simonwuelker commented Mar 9, 2025

#35478 may be relevant.

Yeah right now scripting is always enabled. With this change it is disabled when parsing outside of a browsing context, which exposes servo/html5ever#579.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gterzian gterzian Awaiting requested review from gterzian gterzian will be requested when the pull request is marked ready for review gterzian is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@simonwuelker @jdm