Skip to content

script: Add line number argument to module script compilation functions#55121

Merged
jdm merged 3 commits intoweb-platform-tests:masterfrom
servo:servo_export_39544
Sep 30, 2025
Merged

script: Add line number argument to module script compilation functions#55121
jdm merged 3 commits intoweb-platform-tests:masterfrom
servo:servo_export_39544

Conversation

@servo-wpt-sync
Copy link
Copy Markdown
Collaborator

@servo-wpt-sync servo-wpt-sync commented Sep 28, 2025

Script: added line_no as argument to both fetch_inline_module_script() & compile_module_script() to fix the inline script reporting wrong line issue (#39415).

Originally, the function compile_module_script() hardwires the value 1 when invoking CompileOptionsWrapper::new(). This is fine if the script is written in separate JS file, but for inline scripts, it will cause confusion if the script tag doesn't start from line #1.

line_no is obtained from line_number, a member variable from HTMLScriptElement.

Credits to @jdm for actually pointing out which functions to fix.

Testing: Created a WPT test for this change, specifically: tests/wpt/tests/html/semantics/scripting-1/the-script-element/module/evaluation-error-5.html.

Fixes: issue #39415

Reviewed in servo/servo#39544

Copy link
Copy Markdown
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Servo project.

@servo-wpt-sync servo-wpt-sync changed the title Added new argument line_no to fetch_inline_module_script & compile_module_script to solve issue 39415 script: Add line number argument to module script compilation functions Sep 28, 2025
@servo-wpt-sync
Copy link
Copy Markdown
Collaborator Author

🛠 Changes from the source pull request (servo/servo#39544) can no longer be cleanly applied. Waiting for a new version of these changes downstream.

@servo-wpt-sync servo-wpt-sync force-pushed the servo_export_39544 branch 3 times, most recently from af4498d to e6777a4 Compare September 29, 2025 11:28
@servo-wpt-sync
Copy link
Copy Markdown
Collaborator Author

🛠 Changes from the source pull request (servo/servo#39544) can no longer be cleanly applied. Waiting for a new version of these changes downstream.

…& compile_module_script to fix the inline script reporting wrong line issue (web-platform-tests#39415)

Originally, the function compile_module_script hardwires the value 1 when invoking CompileOptionsWrapper::new(). This is fine if the script is written in separate JS file, but for inline scripts, it will cause confusion if the <script> tag doesn't start from line #1.

Credits to JDM for actually pointing out which functions to fix.

Testing: There are WPT tests for this change, specifically: tests/wpt/tests/html/semantics/scripting-1/the-script-element/module/evaluation-error-5.html
Signed-off-by: RichardTjokroutomo <[email protected]>
Testing: Modified code according to reviewer's comment.
Signed-off-by: RichardTjokroutomo <[email protected]>
…ntics/scripting-1/the-script-element/module/evaluation-error-5.html : 11

Signed-off-by: RichardTjokroutomo <[email protected]>
@servo-wpt-sync servo-wpt-sync added stale-servo-export PRs that were supposed to merge but were not able to do so. and removed do not merge yet labels Sep 30, 2025
@servo-wpt-sync
Copy link
Copy Markdown
Collaborator Author

⛔ The downstream PR has merged (servo/servo#39544), but these changes could not be merged properly. Please address any CI issues and try to merge manually.

@jdm jdm merged commit d674ed8 into web-platform-tests:master Sep 30, 2025
12 of 13 checks passed
@jdm jdm deleted the servo_export_39544 branch September 30, 2025 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

html servo-export stale-servo-export PRs that were supposed to merge but were not able to do so.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants