Closed
Bug 31255
Opened 25 years ago
Closed 24 years ago
JS doesn't ignore --> in HTML (requires // -->); was: the rollover-script for images (left frame) doesn't work
Categories
(Core :: JavaScript Engine, defect, P3)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
M17
People
(Reporter: k.mike, Assigned: rogerl)
References
()
Details
(Whiteboard: [nsbeta2-]ETA 8/01 to review possible Release Note.)
Attachments
(4 files)
From Bugzilla Helper:
User-Agent: Mozilla/4.7 [en] (WinNT; I)
BuildID: 2000022820
http://www.weird-birds.org/
the JavaScript for changing the images in the left frame doesn't work.
the same script works with NS 4.05 and 4.7 plus MSIE 4 and 5. it should change
the images when the event onMouseOver/onMouseOut appears. The script is quite
tricky I know but it's fully JavaScript compatible I think.
Reproducible: Always
Steps to Reproduce:
1. move over any image in the left frame
Actual Results: nothing - this is the bug.
Expected Results: the images should actually change - the orange text should
become green when mouse moves over it...
i implementated the rollover as follows in additional NFO:
<!---- the functions ---->
<script language="JavaScript">
function ein(img_name) {
var src = document[img_name].src;
var off = src.lastIndexOf("_off");
if (off != -1) {
var newsrc = src.substring(0,off) + "_on";
document[img_name].src = newsrc + ".gif";
}
}
function aus(img_name) {
var src = document[img_name].src;
var on = src.lastIndexOf("_on");
if (on != -1) {
var newsrc = src.substring(0,on) + "_off";
document[img_name].src = newsrc + ".gif";
}
}
<script>
<body>
<!--- example line from html-file ---->
<a href="page/creatorz/creatorz.htm" onMouseOver="ein('Creatorz')"
onMouseOut="aus('Creatorz')">
<img name="Creatorz" src="images/Creatorz_off.gif" alt="Info about us!"
border="0"></a>
</body>
|
||
Comment 1•25 years ago
|
||
-> DOM0
Assignee: rogerl → vidur
Component: Javascript Engine → DOM Level 0
QA Contact: rginda → desale
|
||
Comment 2•25 years ago
|
||
Harishd, the problem on this page is that it has a
<script><!--
...
-->
</script>
in it and the JS engine complains when it executes the script because of the
'-->' in the script (note, no '//' in front of the '-->'), do you know how/where
this should be handled? NS 4.x seems happy with such scripts should the JS
engine ignore '-->' or should the parser strip it out or...?
I'll attach a simple testcase...
Assignee: vidur → harishd
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 2000 → All
Hardware: PC → All
|
|
||
Comment 3•25 years ago
|
||
Forgot to mention:
In attachment 7290 [details] <!-- is ended by //-->
In attachment 7291 [details] <!-- is ended by -->
The problem has nothing to do with parser. Looks like a JS engine problem.
Reassigning to rogerl@netscape.com.
Assignee: harishd → rogerl
|
||
Comment 7•25 years ago
|
||
CURRENT STATUS:
Using the latest Mozilla debug build (06/07/00), there is no longer any problem
with the original URL ( http://www.weird-birds.org/) . The text in the left
frame does turn green on MouseOver events. Perhaps the HTML at this site has
been changed?
To see the original problem, it is necessary to use the attachments 7290 and
7291 to this bug. Both of them attempt to wrap JavaScript functions
in HTML comment indicators, as follows:
Attachment 7290 [details] (correct HTML) Attachment 7291 [details] (incorrect HTML)
<SCRIPT> <SCRIPT>
<!-- <!--
(a bunch of JavaScript functions) (a bunch of JavaScript functions)
//--> -->
</SCRIPT> </SCRIPT>
Attachment 7290 [details] works fine in NN4.x, IE4, and Mozilla (the HTML is correct!).
In Attachment 7291 [details], the element '-->' produces an error in the JavaScript
console, because it is not a valid JavaScript comment indicator. This happens in
both NN4.x and the Mozilla build. Despite this error, however, NN4.x and IE4
execute the JavaScript anyway : the text changes to green on MouseOver events.
But in Mozilla, the MouseOver events do not occur...
|
|
||
Comment 8•25 years ago
|
||
Changing component to JavaScript Engine
Component: DOM Level 0 → Javascript Engine
|
||
Comment 9•25 years ago
|
||
IE executes this script because (IIRC) it treats --> as a comment till end of
line, or something close to that. Perhaps we should do the same.
Mozilla differs from Nav4 and earlier because it catches syntax errors before
binding functions, or even compiling code for them. This is within the spec
(ECMA-262). So I think the only question raised by this bug is: should Mozilla
treat --> blah blah, rather than // --> blah blah, as a comment? If yes, should
the JS engine do it (the engine currently does treat <!-- as comment till end of
line, but arguably that non-ECMA check should be in an HTML-specific pre-pass).
/be
Assignee: rogerl → brendan
Summary: the rollover-script for images (left frame) doesn't work → JS doesn't ignore --> in HTML (requires // -->); was: the rollover-script for images (left frame) doesn't work
|
|
||
Updated•25 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → M17
|
||
Comment 10•25 years ago
|
||
The answer to Brendan's first question is no. Imagine what treating --> as a
comment would do to the following, perfectly valid, ECMAScript program:
var x = 10;
var y = 0;
while (x-->0)
y += x;
We should add something like this program to our ECMA test suite.
|
|
||
Comment 11•25 years ago
|
||
Alternatives (can someone play with IE and discover which it uses?):
- Accept --> as a close-comment mark for <!-- within script tags. I thought
this was not valid, because script's content model in HTML is CDATA, which does
not make <, > or anything except </ magic (does not strip SGML comments).
- Accept --> as a comment-till-end-of-line within script tags iff at the start
of a line that occurs after a line beginning with <!--.
- Something else -- ideas?
/be
|
|
||
Comment 12•25 years ago
|
||
*** Bug 40625 has been marked as a duplicate of this bug. ***
|
|
||
Comment 13•25 years ago
|
||
I will add Waldemar's test case ideas to the ECMA test suite -
Bug 40625, which I have marked as a duplicate of this one, had a Status
Whiteboard of "[nsbeta2+][6/15]" and "nsbeta2" as a Keyword. Should we state
the same here, then?
Thanks,
Phil
|
|
||
Comment 14•25 years ago
|
||
Anyone have any thoughts on how to emulate IE, if we should -- or why not,
if we should not?
Phil, can you run waldemar's testcase in IE and report results back here? IE
may use some ugly heuristic such as I proposed (--> at start of a line that
follows a line that began with <!--, or worse). It would be swell to probe for
the algorithm used.
I guess this bug should be nsbeta2+ if the other one was, although the [6/15]
deadline was someone else's idea of when that bug might be fixed. I'm also
bouncing it back to rogerl, now that he has returned from vacation.
/be
Assignee: brendan → rogerl
Status: ASSIGNED → NEW
Whiteboard: [nsbeta2+]
|
|
||
Comment 15•25 years ago
|
||
I have run Waldemar's test case in IE4, NN4.x, and Mozilla as follows:
<HTML>
<SCRIPT>
var x = 10;
var y = 0;
while (x-->0) {
y += x;
document.write('The current value of x is: ' + x + '<BR>');
document.write('The current value of y is: ' + y + '<BR><BR>' );
}
</SCRIPT>
</HTML>
The result was the same in all three browsers:
The current value of x is: 9
The current value of y is: 9
The current value of x is: 8
The current value of y is: 17
.
.
.
The current value of x is: 1
The current value of y is: 45
The current value of x is: 0
The current value of y is: 45
I got no JavaScript errors of any kind -
|
|
||
Comment 16•25 years ago
|
||
Ok -- now change the test so it has a newline just before the -->, and try
again. If that still works, put a <!-- at the top of the SCRIPT tag's content
and try again. Thanks,
/be
|
|
||
Comment 17•25 years ago
|
||
Here is what I've tried now:
<SCRIPT>
var x = 10;
var y = 0;
while (x
-->0) {
y += x;
document.write('The current value of x is: ' + x + '<BR>');
document.write('The current value of y is: ' + y + '<BR><BR>' );
}
</SCRIPT>
This script would not run either in IE4, NN4.x, or Mozilla.
All three times, I got the error "missing ) after condition".
Putting "<!-- in at the top did not change anything -
Let me know if this is not what you had in mind ...
Thanks,
Phil
|
|
||
Comment 18•25 years ago
|
||
That's good to know -- all three implementations conform to ECMA 11.3 (no
LineTerminator may occur between a LeftHandSideExpression and the postfix ++ or
-- operators). This suggests a "fix": let JS treat lines matching /^\s*-->.*$/
as optional leading whitespace (what matched \s*) followed by a comment till end
of line.
Phil, can you see whether
--> hi there
--> hi there
\t--> hi there again
(where \t is a tab character) are ignored by IE? If not, is a line such as
these ignored if there is a <!-- "comment" earlier in the SCRIPT tag's content?
/be
|
|
||
Comment 19•25 years ago
|
||
None of these lines are acceptable in IE4.
I get an error on each one of them:
--> hi there
--> hi there
\t--> hi there again
But I tried some more variations below.
The first script runs in IE4.
The second and third DO NOT RUN in IE4.
NUMBER 1
<SCRIPT>
<!--
document.write('BEFORE');
-->
</SCRIPT>
NUMBER 2
<SCRIPT>
<!--
document.write('BEFORE');
-->
document.write('AFTER');
</SCRIPT>
NUMBER 3
<SCRIPT>
<!--
document.write('BEFORE');
-->document.write('AFTER');
</SCRIPT>
|
Assignee | |
Comment 21•25 years ago
|
||
|
|
Assignee | |
Comment 22•25 years ago
|
||
comments on the patch anyone?
|
|
||
Comment 23•25 years ago
|
||
- Why not do:
if (c != '-')
ts->flags |= TSF_DIRTYLINE;
once just after
if (c == EOF)
RETURN(TOK_EOF);
The cost of an extra not-likely-to-be-taken branch there is a worry, but not
enough of one in light of all the other costs in js_GetToken for me to want
to clone ts->flags |= TSF_DIRTYLINE into all the initial if-then cases.
- Nit: the '\n' case now has multiple statements before its break, so it would
be more consistent with default style to give each statement its own line,
indented four spaces from the switch and two from the case, with an extra
newline after the break. Sorry if my cryptic rules of when to bunch case
statements up on the same line aren't obvious!
- Notice the indentation off-by-one errors (one too many spaces before the case,
one too many before the two lines you added that PeekChar(ts) and test whether
the TSF_DIRTYLINE flag has not been set) below:
+ break;
+
+ case '-':
+ if (MatchChar(ts, '=')) {
+ tp->t_op = JSOP_SUB;
+ c = TOK_ASSIGN;
+ } else if (MatchChar(ts, c)) {
+ if ((PeekChar(ts) == '>') && !(ts->flags & TSF_DIRTYLINE))
+ goto skipline;
+ c = TOK_DEC;
+ } else {
+ tp->t_op = JSOP_NEG;
+ c = TOK_MINUS;
+ }
+ ts->flags |= TSF_DIRTYLINE;
break;
Also, the first && operand is gratuitously parenthesized, but now I'm really
being picky.
Good thing we support more than two chars in ungetbuf!
Thanks, with the above nits picked, r=brendan@mozilla.org
|
|
Assignee | |
Comment 24•25 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
|
|
||
Comment 25•25 years ago
|
||
Verified fixed - using Mozilla tip builds 2000-07-25 on WinNT, Linux.
I checked attachments 7290 and 7291, Waldemar's test case at 2000-06-09 15:49,
and all the HTML test cases following...
Status: RESOLVED → VERIFIED
|
|
||
Comment 26•25 years ago
|
||
Using Mozilla tip builds 2000-07-25 7PM Pacific Time on WinNT, Linux
OOPS - this test case is not working as expected:
<SCRIPT>
<!--
document.write('BEFORE');
-->
</SCRIPT>
The HTML comment-opener "<!--" has long been accepted by JS as a comment
indicator. The fix to this bug is that JS should now accept the HTML
comment-closer "-->" as a JS comment indicator, also.
We would therefore expect the code between the two indicators to execute.
However, Mozilla fails to write 'BEFORE' in the browser window, and produces
a JavaScript error in the console.
Similarly, this test case in the JS shell is failing as follows:
js> eval("<!-- HTM open-comment\nprint('BEFORE');\n-->HTML close-comment");
21: SyntaxError: syntax error:
21: -->HTML close-comment
21: ..^
js>
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
|
||
Comment 27•25 years ago
|
||
Per todays open nsbeta2+ triage - beard believes this may be a corner case, and
that we may be able to Release Note for PR2. Will check back in with beard on
08/01.
Whiteboard: [nsbeta2+] → [nsbeta2+]ETA 8/01 to review possible Release Note.
|
|
||
Comment 28•25 years ago
|
||
Current status:
rogerl has a fix. He will be able to check it in when he returns on Wednesday.
The solution involves checking a type of input that had been overlooked -
|
|
||
Comment 29•25 years ago
|
||
Putting on [nsbeta2-] radar. Too late for beta2.
Whiteboard: [nsbeta2+]ETA 8/01 to review possible Release Note. → [nsbeta2-]ETA 8/01 to review possible Release Note.
|
|
Assignee | |
Comment 30•25 years ago
|
||
Updated fix checked in.
Status: REOPENED → RESOLVED
Closed: 25 years ago → 25 years ago
Resolution: --- → FIXED
|
||
Comment 31•25 years ago
|
||
Adding keyword to bugs which already show a nsbeta2 triage value in the status
whiteboard so the queries don't get screwed up.
Keywords: nsbeta2
this seems to have regressed. For a really minimal testcase try out
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=7675
tested width build 2000-08-13-08 on Win98SE
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
|
|
||
Comment 33•25 years ago
|
||
*** Bug 48962 has been marked as a duplicate of this bug. ***
|
|
Assignee | |
Comment 34•25 years ago
|
||
Aargh, disgusting screw-up. There's one more change to complete the 'final'
updated patch :
--- jsscan.c 2000/08/11 23:51:12 3.32
+++ jsscan.c 2000/08/16 00:05:56
@@ -740,7 +740,7 @@
if (c == EOF)
RETURN(TOK_EOF);
- if (c != '-')
+ if ((c != '-') && (c != '\n'))
ts->flags |= TSF_DIRTYLINE;
hadUnicodeEscape = JS_FALSE;
|
|
||
Comment 35•25 years ago
|
||
So much for my cod review! Sorry about that.
Looks good, but no need for Pascal-precedence parenthesization (ppppppppppppth!)
of == exprs separated by && or ||, eh?
/be
Yep, this seems fixed now. Tried all testcases on all dups and everything works 100%
Verifyed with build 2000-08-17-08 on Win98SE
Status: REOPENED → RESOLVED
Closed: 25 years ago → 24 years ago
Resolution: --- → FIXED
|
|
||
Comment 38•22 years ago
|
||
Testcase added to JS testsuite:
mozilla/js/tests/js1_5/Regress/regress-31255.js
You need to log in
before you can comment on or make changes to this bug.
Description
•