8045 - error locations in *linked* scripts wrong

Status: VERIFIED DUPLICATE of bug 30736

(Core :: DOM: Core & HTML, defect, P3)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Error locations in linked scripts are being given incorrectly (in the text
window, of course).  There are two problems:

1) The filename is given as the name of the main document, not the file
containing the script.  NN4.x got this right.

2) The line number is a line number in the correct file (where the error is),
but it is always one number too small.

Test case will be attached (to work without downloading from bugzilla).

This bug is serious, since it makes writing of demos and testing of the DOM
difficult.  Almost marked as blocker since almost a QA-blocker.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: JavaScript code to cause error

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: HTML that demonstrates incorrect error message using above JavaScript

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The error currently being given is:

JavaScript error: document.doctype has no properties
URL: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=391 LineNo: 2

it should be

JavaScript error: document.doctype has no properties
URL: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=390 LineNo: 3

Comment 4

[John Bandhauer]

1) Changing the component. There is no 'JavaScript Debugger' for mozilla yet.

2) When doing a 'src=' include the engine is supposed to be asked to do a new
eval with a new buffer full of text and with the new filename and lineno param.
Is this not happenning?

joki, are you the right one to look at this? Is this a known bug?

the example has:
<script type="text/javascript" src="/showattachment.cgi?attach_id=390">

Comment 5

[joki (gone)]

Yeah sure, me or Vidur.  I'll take it.

Comment 6

[John Bandhauer]

*** Bug 9710 has been marked as a duplicate of this bug. ***

Comment 7

[John Bandhauer]

I *knew* there was already a bug filed for this. I just couldn't find it. I kind
of like my description - and very simple test - in bug 9710. But it really says
the same thing said here.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Nominating this for [DOGFOOD].  This makes debugging complicated sites near
impossible (see, for example, the recent post on mozilla-layout) and makes it
hard for QA people to figure out the DOM bugs on a given page.

Comment 9

[rickg]

Perhaps we'll make beta.

Comment 10

[rickg]

Moving to m13 because Joki seems to be distracted.

Comment 11

[ekrock's old account (dead)]

Bulk moving [testcase] code to new testcase keyword. Sorry for the spam!

Comment 12

[joki (gone)]

I suspect this won't make M14

Comment 13

[leger]

Putting dogfood in the keyword field.

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I previously nominated this bug for dogfood, and I don't know how I missed it
for beta1 nominations.  Since it seems like the tree will reopen for M15 before
beta1, I'm removing the DOGFOOD and PDT- markers, and nominating for beta1.  I
think one of the goals of a beta is to get developers creating content that
works on the new browser, and this bug is a major obstacle to that.

Comment 15

[daver]

Eric or Rick, can you comment on how much you need this for beta.

Comment 16

[ekrock's old account (dead)]

Painful as it is, we can release note this. Very sorry to inconvenience our 
developers, but I recommend we not PDT+ this for beta1. (Personally, I never pay 
any attention to line numbers. Reporting the filename wrong is much worse, but 
still, it's not an issue that affects consumers or reviewers.)

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

A good place to start looking to fix this bug is the EvaluateScript call at:
http://lxr.mozilla.org/mozilla/source/layout/html/document/src/nsHTMLContentSink.cpp#4122

Comment 18

[John Bandhauer]

I'm attaching a patch that fixes this bug. We can check it in now or later.

NOTE: layout/xml/document/src/nsXMLContentSink.cpp has a similar problem that 
ought to be fixed.

Comment 19

[John Bandhauer]

Attachment: patch to fix bug by passing url along

Comment 20

[leger]

Putting on PDT- radar for beta1.  Putting relnote in keyword field.

Comment 21

[John Bandhauer]

moving to vidur's list. NOTE: patch attached.

Comment 22

[John Bandhauer]

*** Bug 30736 has been marked as a duplicate of this bug. ***

Comment 23

[John Bandhauer]

Note that bug 30736 also has a patch (from norris) to fix this. We should decide 
which one to use and check it in.

Comment 24

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yes, please do check this in... :-).  I was actually about to write a third
patch on 2-18 when you attached yours.  If it means anything, I was going to do
something more similar to what norris did, rather than the little null => use
Document URI hack in EvaluateScript.

This should also be fixed for XML, so don't resolve the bug quite yet...

Note also that the bug marked as a duplicate of this bug was PDT+, so this one
probably should be too...

Comment 25

[Johnny Stenback (:jst)]

Interesting, 30736 should be a dup of this (an AFAIK was marked as such) but
theres no mentioning in 30736 that it would be a dup of this?

Howeve, 30736 is fixed and AFAIK so is this, so I'll mark this a dup of 30736
in stead. (note that the testcase in this bug no longer gives a JS error since
document.documentType is not null any more if a doctype is found in the
document).


*** This bug has been marked as a duplicate of 30736 ***

Comment 26

[Sebastian Späth]

cleaning up: VERIFY duplicate