309712 - HTML-style comments in the middle of .js file cause part of script to be ignored (treated as XML data)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b5) Gecko/20050922
Firefox/1.4

Steps to reproduce:
1. Hover one of the equipment icons in
http://www.thottbot.com/index.cgi?p=Aydsnizzeth.

Result: Nothing happens.

Expected: Info box should appear.

Regression from bug 309242, "E4X should be on by default, while preserving the
comment hiding hack".

Identified by MozillaZine Forum member bizarrojack in
http://forums.mozillazine.org/viewtopic.php?t=321123.

Comment 1

[Jesse Ruderman]

I guess we can...

(1) Back out bug 309242 and go back to requiring opt-in for E4X.

(2) Ask sites that use HTML-style comments in strange ways to change their code.

(3) Change the heuristic introduced by bug 309242 to handle this case as the
author intended.  For example, treat "<!--" as the beginning of XML comment data
iff it does not occur at the beginning of a statement.

Brendan, is (3) realistic?

Comment 2

[Jesse Ruderman]

Attachment: wow.js, the file with the HTML-style in the middle

Copy of http://i73.thottbot.com/js/wow.js.

Comment 3

[Brendan Eich [:brendan]]

Attachment: Use a more conservative heuristic

The heuristic is there just to spare people from having to select an E4X
version, via e4x=1, version=1.6, or better than these ineffective MIME type
parameters on application/javascript, perhaps the application/e4x MIME type
that Chris Lilley is proposing.

So if the heuristic is hurting, make it conservative, which may at the margin
require some E4X users to do the right thing, anyway, and request the MIME type
they require.

/be

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 197152 [details] [diff] [review]
Use a more conservative heuristic

Bah!

r=mrbkap

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 197152 [details] [diff] [review]
Use a more conservative heuristic

sr=shaver, though I'll also ping the Thottbot guys to fix their script!

Comment 6

[Brendan Eich [:brendan]]

Fixed on trunk and branch.

/be

Comment 7

[Brendan Eich [:brendan]]

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

Comment 8

[Jesse Ruderman]

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

Comment 9

[Brendan Eich [:brendan]]

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

Comment 10

[Bob Clary [:bc] (inactive)]

test covered by js1_6/Regress/regress-309242.js