Skip to content

/ servo

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

Jump to bottom

Compiled event handlers and script elements need the source file and line numbers #9604

Closed
jdm opened this issue Feb 11, 2016 · 5 comments
Closed

Compiled event handlers and script elements need the source file and line numbers #9604

jdm opened this issue Feb 11, 2016 · 5 comments
Labels
A-content/dom B-high-value I-papercut

Comments

@jdm
Copy link
Member

@jdm jdm commented Feb 11, 2016

#8491 adds a bunch of TODOs and uses fake values right now.
@jdm jdm added the A-content/dom label Feb 11, 2016
@jdm
Copy link
Member Author

@jdm jdm commented Jun 8, 2016

Also evaluate_script_on_global_with_result passes 0 as the line number argument to CompileOptionsWrapper.
@jdm jdm mentioned this issue Aug 5, 2016
Closed
@jdm
Copy link
Member Author

@jdm jdm commented Aug 5, 2016 •
edited

Elaborating on the last point, from #12744:

We currently set the offset of a script as 0 when its loaded inline.

This means that if an inline script errors, the line number is given relative to the start of the script, not the start of the html file it's in. This is less useful 😄

To fix this, we'll probably need to hook into the parser somehow, not sure. There doesn't seem to be any line number data stored in Servo directly.

(Perhaps have create_element take the line number?)
@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Aug 5, 2016

For relevant Gecko code, search for eTreeOpSetScriptLineNumberAndFreeze.
@Ms2ger Ms2ger changed the title Compiled event handlers need the source file and line numbers Compiled event handlers and script elements need the source file and line numbers Aug 23, 2016
@jdm jdm mentioned this issue Jan 11, 2017
Merged
4 of 4 tasks complete
bors-servo added a commit that referenced this issue Jan 11, 2017
@bors-servo
Auto merge of #14963 - jdm:script_current_line, r=asajeffrey
Loading status checks…
0175fb9 
Report meaningful line numbers for inline script errors

Rebased from #14661.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #12744 and partially #9604
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14963)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jan 12, 2017
@bors-servo
Auto merge of #14963 - jdm:script_current_line, r=asajeffrey
Loading status checks…
52ef8eb 
Report meaningful line numbers for inline script errors

Rebased from #14661.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #12744 and partially #9604
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14963)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Jan 12, 2017
@bors-servo
Auto merge of #14963 - jdm:script_current_line, r=asajeffrey
Loading status checks…
eb72c0e 
Report meaningful line numbers for inline script errors

Rebased from #14661.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #12744 and partially #9604
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14963)
<!-- Reviewable:end -->
@nox
Copy link
Member

@nox nox commented Sep 30, 2017

We support this now, no?
@jdm
Copy link
Member Author

@jdm jdm commented Sep 30, 2017

Yes.
@jdm jdm closed this Sep 30, 2017
@pyfisch pyfisch mentioned this issue Jan 4, 2018
Open
4 of 13 tasks complete
@pshaughn pshaughn mentioned this issue Feb 3, 2020
Open
@jdm jdm mentioned this issue Apr 28, 2020
Merged
4 of 4 tasks complete
bors-servo added a commit that referenced this issue Apr 28, 2020
@bors-servo
Auto merge of #26346 - jdm:console-caller, r=<try>
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
c91e233 
Console caller

These changes use the new API from servo/rust-mozjs#508 to report meaningful filenames and line numbers for APIs that trigger devtools output. They also cause error messages originating from uncompiled event handlers to report a more relevant filename; this differs from Gecko's behaviour, but provides a more useful debugging experience in my opinion.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #9604 and #26344.
- [x] These changes do not require tests because devtools aren't tested.
bors-servo added a commit that referenced this issue Apr 29, 2020
@bors-servo
Auto merge of #26346 - jdm:console-caller, r=SimonSapin
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
02878a6 
Improve devtools output for console APIs

These changes use the new API from servo/rust-mozjs#508 to report meaningful filenames and line numbers for APIs that trigger devtools output. They also cause error messages originating from uncompiled event handlers to report a more relevant filename; this differs from Gecko's behaviour, but provides a more useful debugging experience in my opinion.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #9604 and fix #26344.
- [x] These changes do not require tests because devtools aren't tested.
bors-servo added a commit that referenced this issue Apr 29, 2020
@bors-servo
Auto merge of #26346 - jdm:console-caller, r=Manishearth
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
aa9a8d8 
Improve devtools output for console APIs

These changes use the new API from servo/rust-mozjs#508 to report meaningful filenames and line numbers for APIs that trigger devtools output. They also cause error messages originating from uncompiled event handlers to report a more relevant filename; this differs from Gecko's behaviour, but provides a more useful debugging experience in my opinion.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #9604 and fix #26344.
- [x] These changes do not require tests because devtools aren't tested.
bors-servo added a commit that referenced this issue Apr 29, 2020
@bors-servo
Auto merge of #26346 - jdm:console-caller, r=Manishearth
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
ba0df48 
Improve devtools output for console APIs

These changes use the new API from servo/rust-mozjs#508 to report meaningful filenames and line numbers for APIs that trigger devtools output. They also cause error messages originating from uncompiled event handlers to report a more relevant filename; this differs from Gecko's behaviour, but provides a more useful debugging experience in my opinion.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #9604 and fix #26344.
- [x] These changes do not require tests because devtools aren't tested.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-content/dom B-high-value I-papercut
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

Improve devtools output for console APIs
3 participants
@jdm @Ms2ger @nox
You can’t perform that action at this time.