150584 - JavaScript syntax errors in HTML attributes are not reported.

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Georg Maaß]

JavaScript syntax errors in tag attributes are not reported. If you forget the
closing quotes of a string literal in a JavaScript block this causes an error
message. If you do the same error in a tag attribut, this error is not reported.

Comment 1

[Georg Maaß]

Attachment: Testcase to demonstrate the bug

Comment 2

[Matthias Versen [:Matti]]

-> JS ENGINE (?)

Comment 3

[Phil Schwartau]

Here is the heart of the reporter's testcase above:

<script type="text/javascript">
a='ghj
</script>

<a href="" onclick="b='">
</a>


As the reporter notes, the JS Engine properly reports a syntax error
on the first part(unterminated string literal), but no error is 
generated from the mistake in the link element.

Note the JS Engine can only report errors in <script> elements
or javascript:URLs. The rest of the HTML page, it never sees.

Reassigning to Parser for consideration. Should we, and if so, 
where should we, report HTML syntax errors ???

Comment 4

[harishd]

Our parser is not ready, and may never be ready, to report errors in HTML. If
you really  want errors to be reported use XHTML instead.

Marking WONTFIX.

Comment 5

[Georg Maaß]

Why is this a HTML syntax error? Is this because of not using the ' entity?
But, when using two ', then it works well and is still not a ' entity.

Should such a event handler attribute not be virtually converted by the HTML
parser to something like this:

<script type="DEFAULTTYPE">
HTMLELEMENT.ATTRIBUTNAME = function(event)
{
ATTRIBUTEVALUE
}
</script>

and then given to the JS engine let the JS engine examin whether this is correct
JavaScript or not?

Comment 6

[Georg Maaß]

NN4 and MSIE both report such JS bugs hidden in tag attributes. The let the JS
engine or JScript engine examine the tag attributes content, if it is an
eventhandler tag attribute.

I think this should be done by Mozilla too. The value of event handler
attributes shoult be examined by the engine that is responsible for that
scripting language. In this case it should be sent to the JS engine, which then
would detect and report the error correctly.

Comment 7

[Christopher Hoess (gone)]

There seems to be some confusion about what's being asked for in this bug. AIUI, 
the reporter originally wanted Javascript errors to be reported *for Javascript 
in HTML attributes such as onclick*. This is a matter of JS syntax, not HTML, and 
is not, AFAIK, work for the parser. Bouncing back over to JS Engine--obviously 
scripts in, e.g., <div onclick="this.function"> get fed into there *somehow*.

Comment 8

[Christopher Hoess (gone)]

Reassign.

Comment 9

[Phil Schwartau]

Sorry, I read this one too quickly; you are absolutely correct.
The error <a href="" onclick="b='"> IS a JavaScript syntax error,
not an HTML syntax error as I stated. Again, Georg's testcase is:

<script type="text/javascript">
a='ghj
</script>

<a href="" onclick="b='">
</a>


Note that when I load this in NN4.7, BOTH errors are detected in
the JavaScript Console:

  JavaScript Error:
  http://bugzilla.mozilla.org/attachment.cgi?id=87089&action=view,
  line 10: unterminated string literal. 
           a='ghj
           ..^

  JavaScript Error:
  http://bugzilla.mozilla.org/attachment.cgi?id=87089&action=view,
  line 11: unterminated string literal. 
           b='
           ..^


But in Mozilla, only the first error is detected in the
JavaScript Console. Using trunk binary 20020530xx WinNT.

cc'ing Brendan, Mike, Rob: is this a failure of JS Engine
to report this error?

Comment 10

[Phil Schwartau]

Attachment: Testcase #2

Comment 11

[Phil Schwartau]

cc'ing jst on this, too -

Testcase #2 creates this JavaScript syntax error in the onClick 
attribute of an <input> element:

           <input onClick="javascript:alert('Hi)">

We also add this error handler to the page:

  window.onerror=function(e) {alert('UNEXPECTED ERROR: ' +  e);}


If you load the testcase in NN4.7, you get an alertbox for the error,
plus an error in the JavaScript Console. In IE6, you get the alertbox.
In Mozilla, you don't get anything. Needless to say, when you click
on the input, nothing happens: but you still don't get an error!

Comment 12

[Phil Schwartau]

Note that after I load testcase #2 in Mozilla, if I type that
same error in the URL bar:

                      javascript:alert('Hi)

I get both the alertbox from the window.onerror handler, plus
an error in the JavaScript Console.

Comment 13

[Brendan Eich [:brendan]]

Attachment: proposed fix

This was an oversight from long ago (errors-to-exceptions), and then I
overlooked the JS_Compile*Function* APIs when I fixed bug 97646.  Please test
and review,

/be

Comment 14

[Brendan Eich [:brendan]]

Got the patch, taking the bug.

/be

Comment 15

[Robert Ginda]

Comment on attachment 87291 [details] [diff] [review]
proposed fix

r=rginda

Comment 16

[Johnny Stenback (:jst)]

Comment on attachment 87291 [details] [diff] [review]
proposed fix

sr=jst

Comment 17

[Brendan Eich [:brendan]]

Fix is in the trunk, going for 1.0 branch approval.

/be

Comment 18

[Georg Maaß]

Is this the right folder containing the binaries where this bug is fixed?
ftp://ftp.mozilla.org/pub/mozilla/nightly/2002-06-12-08-trunk/

Comment 19

[Phil Schwartau]

Georg: perhaps ftp://ftp.mozilla.org/pub/mozilla/nightly/2002-06-12-08-trunk/
is a little too soon to contain the fix. I don't know when they pulled source
to make those builds. 

At any rate, the WinNT build from that ftp directory shows no improvement.
Let's try the next set of builds when they come out -

Comment 20

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

Comment on attachment 87291 [details] [diff] [review]
proposed fix

Please land this on the 1.0.1 branch.  Once there, replace the
"mozilla1.0.1+" keyword with the "fixed1.0.1" keyword.

Comment 21

[Phil Schwartau]

Using Mozilla trunk binary 2002061408 on WinNT.

With this fix, a JS syntax error in an onClick handler IS now reported
in the JS Console, but only when the user actually clicks on the link.
Similarly, a window.onerror only occurs on the actual click.

That makes sense to me, but this differs from the behavior of both
NN4.7 and IE6, where a JS syntax error in an onClick handler is caught
on page load (just try testcase #2 in all three browsers).

Question for the developers: are we happy with this?

Comment 22

[Johnny Stenback (:jst)]

We lazily compile event handlers in mozilla, that's why you see the errors only
when clicking... That won't change just because it'd be nice to see errors
earlier...

Comment 23

[Georg Maaß]

Error messages are not to make users happy but to inform developers about 
nonscence they made. So this should be changed some day.

Comment 24

[Johnny Stenback (:jst)]

There's a fairly large memory useage cost we'd need to pay if we don't compile
the event handlers lazily, and I doubt that's acceptable given the current
memory footprint.

Comment 25

[Georg Maaß]

Then you may plan for future a preference to switch off laziness, where web
developers prefere quick error detection in favour of more efficient memory
usage. Default may be lazyness. But an option to force error detection when
loading the document, would be nice.

Comment 26

[Brendan Eich [:brendan]]

Fixed on the branch.

/be

Comment 27

[Phil Schwartau]

Patch verified on the 1.0 branch as well -