238945 - do;while(0)return needs autosemicolon insertion (JS generated table not shown, works in 1.4.1 and 1.6)

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Hermann Schwab]

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7b) Gecko/20040326
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7b) Gecko/20040326

The URL is the testcase of Bug 216065, crashing in Mozilla 1.5b when sorting the
table.
I couldn´t see the table in current nightly and Mozilla 1.7b, saw it in 1.4.1,
and saw it and tested sorting in Mozilla 1.6. 
1.6 based Firefox 0.8 also shows the table, current nightly of Firefox doesn´t.

Javascript Console generates errors using Mozilla 1.7b, none with 1.4.1 or 1.6.
1.7b shows the table using a local copy of the website made with 1.6.
1.6 saves 10 files, 1.7b only 7 files.

Reproducible: Always
Steps to Reproduce:
1.Load URL using Mozilla 1.4.1, 1.6 or Firefox 0.8, see the table, save complete
2. Load URL using Mozilla 1.7b or current nightly, don´t see table, save complete
3. Using Mozilla 1.7b compare saved pages.

Actual Results:  
Mozilla 1.4.1, 1.6 and Firefox 0.8 didn´t show errors on the Javascript Console,
displayed the table.
Mozilla 1.7b and current nightlies don´t show the table, show errors on the
Javascript Console, save less files than the older browsers, but display
correctly the files saved using the older browsers.


Expected Results:  
no regression, working like Mozilla 1.4.1 and 1.6 or Firefox 0.8 resp.

Comment 1

[Hermann Schwab]

working: BuildID 2004010408,   regressed BuildID 2004010508
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7a) Gecko/20040105

checkins in that time frame:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-01-04+07%3A00&maxdate=2004-01-05+09%3A00&cvsroot=%2Fcvsroot

changes to jsparse.c, no bugnumber:
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/js/src&command=DIFF_FRAMESET&file=jsparse.c&rev1=3.93&rev2=3.94&root=/cvsroot



JS console was showing warnings only in the working Mozilla,
is additionally showing three errors in the regressed build.

Error: missing ; before statement
Source File: http://www.softcomplex.com/products/tigra_tables_pro/ttp_files/table.js
Line: 13, Column: 200
Source Code:
of(TT1L[TT1F[TT1O][1]])!='function')TT1R=new String(TT1F[TT1O][1](TT1L));else
TT1R=new
String(TT1L[TT1F[TT1O][1]]());if(TT1O=='h')TT1I=TT1L.getHours();TT2+=TT1R}else
TT2+=TT1O;i++}while(i<TT1N.length)return TT2};function TT06(TT1N,TT1S){var
TT1T=[],TT1O,T

Error: TTz is not defined
Source File:
http://www.softcomplex.com/products/tigra_tables_pro/ttp_files/template.js
Line: 1

Error: TTable is not defined
Source File: http://www.softcomplex.com/products/tigra_tables_pro/demo1.html
Line: 100

Comment 2

[Brendan Eich [:brendan]]

The test URL for this bug does not conform to ECMA-262 Edition 3, section 7.9
"Automatic Semicolon Insertion".  See 7.9.1 first bulleted item condition 1.

But it seems everyone allows do{}while(0)return to be run together on one line.
 Does IE alow it?  Just curious, really I think I should not have enforced ECMA
purity in a way that breaks real-world JS.  I'll patch for 1.7.

Narcissus will need a patch too.

/be

Comment 3

[Brendan Eich [:brendan]]

Hermann, any chance you are involved in the test URL and can get it fixed to
work with the ECMA spec?  That seems better in the long run.

I was hoping that the html file including the table.js script used a non-ECMA
JavaScript version, but it doesn't, so I would have to make an unconditional
deviation from ECMA in the jsparse.c code, something we try to avoid.

/be

Comment 4

[Brendan Eich [:brendan]]

Attachment: possible fix, need more discussion, also info from reporter

If the only JS in the world that expects this were at the test URL, and we
could get it changed to follow ECMA, then I would favor that.  Otherwise, this
patch is the best compromise.

/be

Comment 5

[Hermann Schwab]

(In reply to comment #3)
> Hermann, any chance you are involved in the test URL and can get it fixed to
> work with the ECMA spec?  That seems better in the long run.

No, I´m not involved, I found it looking at the testcase of Bug 216065
Also I don´t know enough about JS, but thought it´s creator would know ;-)

> I was hoping that the html file including the table.js script used a non-ECMA
> JavaScript version, but it doesn't, so I would have to make an unconditional
> deviation from ECMA in the jsparse.c code, something we try to avoid.
> 
> /be

I´m glad I was right to put you on CC ;-)

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 145039 [details] [diff] [review]
possible fix, need more discussion, also info from reporter

I've got the equivalent patch for narcissus ready.

/be

Comment 7

[Mike Shaver (:shaver emeritus)]

Comment on attachment 145039 [details] [diff] [review]
possible fix, need more discussion, also info from reporter

r=shaver. This is probably the right fix for 1.7, and we can revisit our purity
stance on this at a later date.  (More likely: never.)

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 145039 [details] [diff] [review]
possible fix, need more discussion, also info from reporter

Gotta unregress 1.7.

/be

Comment 9

[chris hofmann]

Comment on attachment 145039 [details] [diff] [review]
possible fix, need more discussion, also info from reporter

a=chofmann for 1.7

Comment 10

[Brendan Eich [:brendan]]

Fixed, thanks all!

/be

Comment 11

[Hermann Schwab]

Thanks, wfm Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7b) Gecko/20040330

Comment 12

[Brendan Eich [:brendan]]

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

Comment 13

[Bob Clary [:bc] (inactive)]

Attachment: js1_5/Regress/regress-238945.js

Hermann, with your permission this will be included in the javascript test
library.

Comment 14

[Hermann Schwab]

(In reply to comment #13)
> Created an attachment (id=174970) [edit]
> js1_5/Regress/regress-238945.js
> 
> Hermann, with your permission this will be included in the javascript test
> library.

Thanks for the honor, permission granted.

Comment 15

[Bob Clary [:bc] (inactive)]

js/tests/js1_5/Regress/regress-238945.js checked in.