255895 - JS1.2 must die (was: Page with menus looks good in milestones and older releases; fails with recent nightlies)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Neil Parks]

User-Agent:       Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.7) Gecko/20040803 Firefox/0.9.3
Build Identifier: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.7) Gecko/20040803 Firefox/0.9.3

Please compare http://www.fuchsmizrachi.org in Firefox 0.9.3 vs. recent FF
branch nightlies such as Aug 11 and Aug 16.

The page looks good in the milestone (and older versions of FF).  With the
recent nightlies, the menus in the upper left do not appear.

See also this thread in the Firefox Bugs forum:
http://forums.mozillazine.org/viewtopic.php?t=114796&highlight=




Reproducible: Always
Steps to Reproduce:
1.  Visit http://www.fuchsmizrachi.org with milestone 0.9.3.
2.  Visit same with recent branch nightly.


Actual Results:  
Page looks very good with milestone.  Menus are missing with nightly.


See discussion in Firefox Bugs forum:

http://forums.mozillazine.org/viewtopic.php?t=114796&highlight=

Comment 1

[Martijn Wargers (dead)]

I'll summarize what I said in that forum topic.
This seems like a regression of fixing bug 248549, regressed in trunk builds
from 2004/07/23 and later.
The menu script uses:
 ie=(document.all != (void 0)); 
which evaluates to true from 2004/07/23 and later on. Before that it evaluated
to false. (void 0) is equal to undefined.

See also bug 248549, comment 49. The assumption made in that comment, seems at
least in this case to be wrong (however, in general this is probably true).

Comment 2

[Martijn Wargers (dead)]

This is the link to the script that generates the menu and which uses
'ie=(document.all != (void 0));':
http://www.fuchsmizrachi.org/popouttext.js

Comment 3

[Stephen Deken]

I suspect that this is a byproduct of bug 248549, which exposes document.all
unless it is detected first.  My hypothesis is that the syntax:

     ie = (document.all != (void 0));

is taken to be a *use* of document.all, which makes newer builds expose it, and
therefore the expression evaluates to true.

Comment 4

[Bob Clary [:bc] (inactive)]

interesting if unconventional detection code. I have contacted the site and hope
to hear back soon.

I think we can treat this as evangelism unless Brendan wishes to support the
comparison against (void 0).

Comment 5

[Brendan Eich [:brendan]]

I'd like not to cope with (void 0), because I bet after bloating
jsobj.c:Detecting with code to match the JS bytecode generated for that special
form, there'll be a variation that requires smarter matching, or just more
brute-force matching.

If this comes up again, though, please confirm and dup.

/be

Comment 6

[Neil Parks]

Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.7.2) Gecko/20040827 Firefox/0.9.1+

I presume this release contains changes made for bug #256932 ?

Menus now appear when page is loaded--but hovering over them produces garbage
instead of submenus.

Comment 7

[Bob Clary [:bc] (inactive)]

Neil, yes I believe the recent 0.9.x nightly builds of FF do contain the
undetected document.all. I have written to the site explaining the situation and
asking them to change ie = (document.all != (void 0)) to ie = (document.all !=
null), however I have not heard back from them. Perhaps if you could contact
them it would help.

This use of (void 0) in a detection scheme is rather unique. The addition of the
undetected document.all provides significant improvement on older sites and
sites which are coded solely for MSIE and which do not use significant MSIE
features. It is unfortunate that these changes have broken this particular site,
but I still believe that the benefit outweighs the cost.

Comment 8

[chris hofmann]

minus for PR,  watch for additional feedback...

Comment 9

[Martijn Wargers (dead)]

This is the place where these pop-out menus gets sold:
http://www.thetemplatestore.com/popoutmenu/
It might be a good idea to contact the author of that menu, that Mozilla's
behavior has changed regarding document.all.

(Thanks to danm,
http://forums.mozillazine.org/viewtopic.php?t=127993&sid=ea8f063553798a80263cb24104570c44
)

Comment 10

[Bob Clary [:bc] (inactive)]

contacted [e-mail]

Comment 11

[chris hofmann]

so far, document.all changes seem to be holding up ok.  -minusing this bug, and
hoping we can evangelize the few sites where we have problems.

Comment 12

[Bob Clary [:bc] (inactive)]

I have contacted the site. The problem currently is the fact that when
javascript1.2 is specified as the script language, == is treated as === and !=
is treated as !==

  if (document.all != null)
  {
     // detected in js1.2
  }

I don't know what to tell the author. I consider the js1.2 emulation stuff to be
a potential significant problem with document.all detection.

Comment 13

[chris hofmann]

brendan, jst,  ideas?

Comment 14

[Brendan Eich [:brendan]]

Yeah, we need to give up our hi-fidelity JavaScript1.2 compatibility, at least
as far as == and != are concerned, and go with the IE flow.  I guess this is mine.

/be

Comment 15

[Erik Fabert]

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

Comment 16

[Bob Clary [:bc] (inactive)]

updated menu available at
<http://www.ecommercetemplates.com/free_downloads.asp>. I contacted the site
about updating.

Comment 17

[Brendan Eich [:brendan]]

bc, anyone: what, if anything, does IE do differently given
language=JavaScript1.2 instead of language=JavaScript ?

/be

Comment 18

[Bob Clary [:bc] (inactive)]

I can not find any documentation on msdn that would leave me to believe that the
behavior changes dependent on the value of the script tag's javascript version.
I attempted to investigate using the experimental online js tests and found some
minor differences in the reports however am not certain the differences are due
to the browser or due to the the way the tests are executed. I will continue to
attempt to quantify the situation and will report back when I have more information.

Comment 19

[chris hofmann]

time is growing short for RC1

Comment 20

[Brendan Eich [:brendan]]

Time is short for anything releasing soon.  This kind of change (which I would
make in the content code, not in the core JS engine -- messing with <script
language="JavaScript1.2">'s interpretation, not with JSVERSION_1_2 in jspubtd.h)
should really be made on the trunk well before any product release.

Data from Bob is what I crave, and I'm in his debt already for document.all data
he's collected.  Unless he thinks making JSVERSION_1_2 == JSVERSION_DEFAULT (in
the content code, for HTML only) is a slam dunk, I'm happy to withdraw my 1.0+
mark and do this on the trunk (soon, but no rush).

/be

Comment 21

[Bob Clary [:bc] (inactive)]

I don't feel comfortable with this change so late in the cycle. I don't think we
have seen many bugs filed with document.all detection against null in
javascript1.2 and can live with the javascript1.2 differences for FF1.0. I will
have better test results for MSIE, Mozilla and Opera for different language
settings soon and we can make a more informed decision for post FF1.0.

Comment 22

[chris hofmann]

in talking with bob and brendan on this I going to minus for 1.0.  we should get
a change for this on the trunk and let it go throught a 1.8 alpha and beta. 
probably not a lot of risk in making this change, but it would to have it go
though a good testing cycle...

Comment 23

[Erik Fabert]

Attachment: language="JavaScript 1.2" vs language="JavaScript"

I searched lxr for JSVERSION_1_2 and, if I understood what was going on, made a
test case. In all cases, Mozilla returns a different result between JS 1.2 and
JS 1.5. IE6, otoh, doesn't care about language="JavaScript1.2", it always
returns the JS 1.5 result (same for Opera 7.5).

It looks like removing all the special treatment for 1.2 (except for getYear(),
probably) would be reasonably safe.

Comment 24

[Brendan Eich [:brendan]]

Attachment: nullify version selection done via <script language="JavaScript1.x"> in HTML

Bob, feel free to test the heck out of this patch.  Thanks,

/be

Comment 25

[Johnny Stenback (:jst)]

Comment on attachment 174116 [details] [diff] [review]
nullify version selection done via <script language="JavaScript1.x"> in HTML

r+sr=jst

Comment 26

[Bob Clary [:bc] (inactive)]

Attachment: list of non default javascript version external scripts found in top70 sites

The patch still respects versions 1.1 to 1.5 does not run versions greater than
1.5 which is a good thing(tm). When running the online javascript tests, the
javascript version 1.2 behaves the same as the default.

Question: is the default javascript version the same as specifying
javascript1.5? I thought there was some difference between when the user
specified language="javascript" and language="javascript1.5".

I ran scans of the top 70 sites before and after this patch. I did not find any
obvious problems so far in over 4000 pages. The majority of non default version
javascript external files have to do with either hitbox or coremetrics and are
in javascript1.1

I think this needs to start baking as soon as possible to flush out any
problems.

Comment 27

[Brendan Eich [:brendan]]

> Question: is the default javascript version the same as specifying
> javascript1.5? I thought there was some difference between when the user
> specified language="javascript" and language="javascript1.5".

There's no difference, because there are no runtime version checks against
JSVERSION_1_5, only JSVERSION_IS_ECMA tests (which is a macro that does this:

#define JSVERSION_IS_ECMA(version) \
    ((version) == JSVERSION_DEFAULT || (version) >= JSVERSION_1_3)

There are a few JSVERSION_1_4 tests, but they all use relational operators to
try to balance ECMA Edition 2 compatibility.  See
http://lxr.mozilla.org/mozilla/search?string=JSVERSION_1_4 for details.  From
jsconfig.h:

/*
 * Compile-time JS version configuration.  The JS version numbers lie on the
 * number line like so:
 *
 *      1.0     1.1     1.2     1.3     1.4     ECMAv3  1.5
 *              ^                       ^
 *              |                       |
 *              basis for ECMAv1        close to ECMAv2
 *
 * where ECMAv3 stands for ECMA-262 Edition 3.  See the runtime version enum
 * JSVersion in jspubtd.h.  Code in the engine can therefore count on version
 * <= JSVERSION_1_4 to mean "before the Third Edition of ECMA-262" and version
 * > JSVERSION_1_4 to mean "at or after the Third Edition".
 *
 * In the unlikely event that SpiderMonkey ever implements JavaScript 2.0, or
 * ECMA-262 Edition 4 (JS2 without certain extensions), the version number to
 * use would be near 200, or greater.
 *
 * The JS_VERSION_ECMA_3 version is the minimal configuration conforming to
 * the ECMA-262 Edition 3 specification.  Use it for minimal embeddings, where
 * you're sure you don't need any of the extensions disabled in this version.
 * In order to facilitate testing, JS_HAS_OBJ_PROTO_PROP is defined as part of
 * the JS_VERSION_ECMA_3_TEST version.
 */

I committed the patch, so we're baking for 1.8b2, hoping to make 1.8final and
derived products (Firefox 1.1) that much more compatible with other browsers.

/be

Comment 28

[Neil Parks]

(In reply to comment #6)
> Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.7.2) Gecko/20040827
Firefox/0.9.1+
> 
> I presume this release contains changes made for bug #256932 ?
> 
> Menus now appear when page is loaded--but hovering over them produces garbage
> instead of submenus.

Not fixed in Deer Park Alpha 1
Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.8b2) Gecko/20050531 Firefox/1.0+

Comment 29

[Bob Clary [:bc] (inactive)]

https://bugzilla.mozilla.org/attachment.cgi?id=167555 shows js12 === js15.

the issue with the menu is they are testing document.all == void 0 which returns
false regardless of the language version. getting them to change to compare
against null will fix the site. Please contact them and/or file a tech
evangelism bug on them.

the issue for this bug (that language="javascript1.x" is treated as
language="javascript") is fixed.

Comment 30

[Yuh-Ruey Chen]

Attachment: screenshot of test page in fx 1.0.x

for historical purposes