Closed Bug 238945 Opened 20 years ago Closed 20 years ago

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

Categories

(Core :: JavaScript Engine, defect, P1)

x86
Windows 98
defect

Tracking

()

VERIFIED FIXED
mozilla1.7final

People

(Reporter: hhschwab, Assigned: brendan)

References

()

Details

(Keywords: js1.5, regression)

Attachments

(2 files)

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.
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
Component: Browser-General → JavaScript Engine
Keywords: regression
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
Assignee: general → brendan
Keywords: js1.5
Priority: -- → P1
QA Contact: general → pschwartau
Target Milestone: --- → mozilla1.7final
Status: NEW → ASSIGNED
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
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
(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 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
Attachment #145039 - Flags: review?(shaver)
Summary: JS generated table not shown, works in 1.4.1 and 1.6 → do-while(0)return needs autosemicolon insertion (JS generated table not shown, works in 1.4.1 and 1.6)
Summary: do-while(0)return needs autosemicolon insertion (JS generated table not shown, works in 1.4.1 and 1.6) → do;while(0)return needs autosemicolon insertion (JS generated table not shown, works in 1.4.1 and 1.6)
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.)
Attachment #145039 - Flags: review?(shaver) → review+
Comment on attachment 145039 [details] [diff] [review]
possible fix, need more discussion, also info from reporter

Gotta unregress 1.7.

/be
Attachment #145039 - Flags: approval1.7?
Flags: blocking1.7+
Comment on attachment 145039 [details] [diff] [review]
possible fix, need more discussion, also info from reporter

a=chofmann for 1.7
Attachment #145039 - Flags: approval1.7? → approval1.7+
Fixed, thanks all!

/be
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Thanks, wfm Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7b) Gecko/20040330
Status: RESOLVED → VERIFIED
*** Bug 239315 has been marked as a duplicate of this bug. ***
Hermann, with your permission this will be included in the javascript test
library.
(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.
js/tests/js1_5/Regress/regress-238945.js checked in.
Flags: testcase+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: