Closed Bug 440595 Opened 17 years ago Closed 16 years ago

[FIX]Line numbers are incorrectly reported in onerror

Categories

(Core :: General, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.9.1b2

People

(Reporter: bedney, Assigned: bzbarsky)

References

(Depends on 1 open bug)

Details

(Keywords: regression, testcase, verified1.9.1)

Attachments

(4 files)

User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008061004 Firefox/3.0 Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9) Gecko/2008061004 Firefox/3.0 When a JavaScript parsing error occurs in JavaScript which has been referenced via a 'src' in a <script> tag, and you have supplied your own 'onerror' handler, the line number supplied to the onerror handler is always 0, even though the proper line number is reported in the Error Console. See the attached files. Reproducible: Always Steps to Reproduce: 1. Run the test files 3. The error console shows that the line number is 7, which is correct. Actual Results: The alert panel shows that the line number is 0, which is incorrect. Adding or removing code from onerrortest.js will show that the line number is always reported to the onerror handler as 0. Expected Results: The alert panel should show that the error occurred on line 7, which would be the correct value. The error console shows this. Note that Firefox 2.0 reports the line number correctly. This is something that got changed in Firefox 3.0.
Attached file The HTML file showing the bug. (deleted) —
One other thing that should be mentioned here is that if the JavaScript is 'inlined' in the HTML page (i.e. it is *not* referenced from an external file), then the line number is correctly reported. Cheers, - Bill
Blocks: 402983
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression, testcase
OS: Mac OS X → All
Product: Firefox → Core
QA Contact: general → general
Hardware: Macintosh → All
Version: unspecified → Trunk
Flags: blocking1.9.1?
Flags: blocking1.9.0.1?
Does this happen when the files are loaded from a webserver? Or only when loaded from the local file system? If the former then the regression range sounds unlikely (though of course not impossible)
Jonas - Thanks for the quick response. I tested it again and it only happens when they are loaded from the local file system. Now, some of us develop, launch and load entire Web / AJAX applications and run them completely from the file system ;-), so this is of concern to us. Thanks! Cheers, - Bill
Then the dependency is most likely correct. Sounds like a bad regression.
Assignee: nobody → dveditz
Flags: wanted1.9.0.x+
Flags: blocking1.9.1?
Flags: blocking1.9.1+
Priority: -- → P1
In Fx3.0 third argument in the event handler "window.onerror" does not return a proper line number (always "0") on local external scripts (not-html but own .js file like <script src=url>). Also, the first argument does not show the real problem but only text "Script error" (before in Fx2.0 it used to show i.e. "variable asdfj does not exist" etc.).
Flags: blocking1.9.0.1? → blocking1.9.0.1-
So the issue is that we do a same-origin compare on the URI of the loading page and the loaded script. Since each file is now a separate origin, they are not same-origin, and hence we're not allowed to leak information (such as line numbers of errors, or the actual error text) to the onerror handler. We'd have to actually keep track of the script's origin principal (instead of just the URI) to fix this... Well, and pass loading principals into the scriptloader too. We have a FIXME on this in the code. Alternately, we could try skipping the check for file:// URIs in nsJSenvironment, or replacing it with a CheckMayLoad check. In fact, the CheckMayLoad approach may be the right one in general, instead of using CheckSameOriginURI.
Attached patch Like so (deleted) — Splinter Review
Assignee: dveditz → bzbarsky
Status: NEW → ASSIGNED
Attachment #331615 - Flags: superreview?(jst)
Attachment #331615 - Flags: review?(jonas)
Depends on: 424484
Summary: Line numbers are incorrectly reported in onerror → [FIX]Line numbers are incorrectly reported in onerror
Comment on attachment 331615 [details] [diff] [review] Like so I *think* this is right, but please make sure jst double-checks that this doesn't revert the original intention of this code. The only thing that is non-ideal is that you are sort of abusing the API here since you are not actually loading anything, right. Ideally you'd want p->Subsumes(errorPrincipal) i think, but that's what the comment is about. Actually, if that is indeed the case please add that to the comment.
Though I guess it's sort of related to loading since you are handing out information related to data loaded from the particular URI...
Comment on attachment 331615 [details] [diff] [review] Like so Yeah, this does look right. sr=jst
Attachment #331615 - Flags: superreview?(jst) → superreview+
Pushed changeset 3762e6244e63. No test yet.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Comment on attachment 331615 [details] [diff] [review] Like so Probably worth fixing on branch too.
Attachment #331615 - Flags: approval1.9.0.4?
Comment on attachment 331615 [details] [diff] [review] Like so Without a test, this can wait for 3.1 given the new more-restrictive branch policy.
Attachment #331615 - Flags: approval1.9.0.4? → approval1.9.0.4-
Uh... we do have a test, and I did test this in a branch build. We just have no way of running the test automatically on branch (or trunk, for that matter). And of course this was a Fx3 regression that significantly complicates the job of web developers who develop against a file:// tree. I'm OK with not taking this on branch as long as we're on the same page on the above.
Keywords: fixed1.9.1
Target Milestone: --- → mozilla1.9.1b2
Verified fix from comment 0 on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090217 Minefield/3.2a1pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090217 Shiretoko/3.1b3pre Ubiquity/0.1.5. Error console and alert both show the error on line number 7.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: