Closed Bug 253150 Opened 18 years ago Closed 18 years ago



(Core :: JavaScript Engine, defect, P1)






(Reporter: brendan, Assigned: brendan)


(Keywords: fixed-aviary1.0, fixed1.7, js1.5)


(2 files, 1 obsolete file)

1.  JSOP_TYPEOF should be considered to be "detecting".
2.  The strict warning "reference to undefined property foo" given for
should not be given for detecting cases.

See bug 248549 comment 50.

Will you be able to make (document.all == null) == true and (document.all !=
null) == false as well? ditto undefined.
Patch after I get lunch.

Flags: blocking-aviary1.0PR?
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta
bc: I'll try -- no promises.  Do you actually see such tests in pages?  How
many, out of total that detect document.all?

I noticed this over the weekend in <> in an old Macromedia
Dreamweaver function MM_showHideLayers() v 2.0. Note the page also contains a
commented out newer version 3.0 of the same function which does not use the
document.all != null test. We actually work with the v 2.0 script because of the
document.all != null test. Even though I don't have a list of sites which are
broken because of this, it still worries me a bit since these cases could result
in our going down the IE path in inappropriate circumstances. 

If we can't fix it, I would really like to hear from Macromedia/Adobe/Oracle/IBM
etc to make sure this doesn't cause them problems.

Looking in google:

"document.all != null" OR "document.all == null" 
shows a "few" (642 English pages/1,060 Total) pages, 

"document.all != undefined"  OR "document.all == undefined" also is small (132
English/139 total) 

"typeof(document.all) == 'undefined'"  OR "typeof(document.all) == 'undefined'"
is smaller still (123 English/130 Total).

These are tiny relative to the "massive" (506,000 English/607,000 Total) pages
returned for "document.all", but could include important edge cases not easily
found using the approaches I have been using.

So, my recommendation is if we can make this truly "undetecting" we should.
Does anyone use document.all === undefined or document.all !== undefined?  I'd
imagine the number would be similarly small to some of these other cases.
I don't have any data on the usage of strict equality operators, but feel it is
pretty uncommon.
Attached patch proposed patch (obsolete) — Splinter Review
This handles document.all == null and document.all != null, but not undefined,
and not the strict equality ops.

Handling undefined would take more work, because you have to look up undefined
at runtime after recognizing JSOP_NAME <undefined> in the bytecode, in lieu of
JSOP_NULL.  The undefined property of the global object was not added till
ECMA-262 Edition 3, and it's read/write, so script can redefine it.

This patch adds 76 bytes of -O2 Makefile.ref text size to jsobj.o with

Attachment #154430 - Flags: superreview?(jst)
Attachment #154430 - Flags: review?(shaver)
Comment on attachment 154430 [details] [diff] [review]
proposed patch

Better patch, this is not extensive enough, and worse, the JSOP_NULL;
JSOP_EQ|JSOP_NE test is busted!

Attachment #154430 - Attachment is obsolete: true
Attachment #154430 - Flags: superreview?(jst)
Attachment #154430 - Flags: review?(shaver)
Attached patch better patchSplinter Review
This handles document.all tests againts null using == and !=, and against
undefined using ==, !=, ===, !===.  It does not worry about JavaScript1.2.  It
does not worry about the possibility that someone might redefine undefined.  It
doesn't worry about tests against void 0 or void (any expression).

You can play with it in the js shell using the built-in "it" object, by setting
it.noisy to true:

js> it.noisy=true
js> it.all == null
resolving its property all, flags {qualified,,detecting}
getting its property all, current value undefined
js> it.all != null
resolving its property all, flags {qualified,,detecting}
getting its property all, current value undefined
js> it.all == undefined
resolving its property all, flags {qualified,,detecting}
getting its property all, current value undefined
js> it.all != undefined
resolving its property all, flags {qualified,,detecting}
getting its property all, current value undefined
js> it.all === undefined
resolving its property all, flags {qualified,,detecting}
getting its property all, current value undefined
js> it.all !== undefined
resolving its property all, flags {qualified,,detecting}
getting its property all, current value undefined
js> it.all !== void 0
resolving its property all, flags {qualified,,}
getting its property all, current value undefined

Comment on attachment 154528 [details] [diff] [review]
better patch

Bob is gonna test this patch, but feel free to review starting now :-).

Attachment #154528 - Flags: superreview?(jst)
Attachment #154528 - Flags: review?(shaver)
Comment on attachment 154528 [details] [diff] [review]
better patch

Attachment #154528 - Flags: superreview?(jst) → superreview+
Attached file zipped test results
The zip file contains the full results. Here is the summary.

Spidered home pages of test sites list from bug 248549 
combined with top 70 sites using Spider
with user hooks
which outputs Quirks/Standards mode, OBJECT/EMBED information and
injects mouse[over|out] pairs on each element.

CSS Parsing Errors and JavaScript Strict Warnings, and
JavaScript Chrome Errors were enabled.

628 pages attempted
-73 failed 
555 tested

Quirks vs. Standards Mode
505 pages in Quirks mode

undetected document.all

23 occurences of Non-standard document.all in 22 pages.
All occur in pages in Quirks mode.

 2 JavaScript Exceptions involving document.all

  JavaScript Exception: document.all.namedItem is not a function. 
  Source File:, Line: 166.
  JavaScript Exception: document.all.plazadiv[kkk] has no properties. 
  Source File:, Line: 2845.
  partly fixed. mixed object detection.
  not fixed. nn4/ie4 detection.
  mostly works, detects nn4 then assumes ie4. minor global id var issue
  worked already, coded to be cross browser though uses some MSIE only 
  features which are detected. document.all non-standard message may be 
  not fixed, uses HTMLElement.parentElement
  fixed. site is ie5+/gecko using XMLHttpRequest etc. The document.all error
  was just an oversight.
  not fixed. HTMLElement.parentElement.
  not fixed due to nn4/ie4 detection.
  partly fixed. document.all.namedItem,
  partly fixed, nn4/ie4 detection, window.event

The remaining sites I leave as an exercise for the reader.


22551 JavaScript Warnings of which 10834 are unique
 3078 JavaScript Exceptions of which 194 are unique
   40 CSS Warnings of which 33 are unique
 8295 CSS Errors of which 2645 are unique

See bug253150.log for details and error-summary.txt for a summary.

Plugins and ActiveX

14 WARNING: OBJECT Tag has FLASH WMODE but child EMBED Tag does not.
11 WARNING: OBJECT Tag does not contain EMBED Tag

clsid:d27cdb6e-ae6d-11cf-96b8-444553540000 occurred 135 times.
clsid:fdc7a535-4070-4b92-a0ea-d9994bcc0dc5 occurred 2 times.
clsid:aaf15a90-f3ec-4fee-9a00-f65b25b83d05 occurred 2 times.
clsid:6bf52a52-394a-11d3-b153-00c04f79faa6 occurred 1 times.
clsid:575a1237-563b-11d4-9105-00105aa4854f occurred 1 times.

d27cdb6e-ae6d-11cf-96b8-444553540000 Flash
fdc7a535-4070-4b92-a0ea-d9994bcc0dc5 RealOnePlayer
aaf15a90-f3ec-4fee-9a00-f65b25b83d05 ESPN DIGStreamClientInfo
6bf52a52-394a-11d3-b153-00c04f79faa6 Windows Media Player 7+
Just for the record, here is a site that uses "typeof" to detect IE (main menu
is now broken):
I expect it to be fixed along with this bug.
Comment on attachment 154528 [details] [diff] [review]
better patch

I'd like a few comments in Detecting describing the script that would
correspond to the different cases, but r=shaver anyway because I'm a soft
Attachment #154528 - Flags: review?(shaver) → review+
Comment on attachment 154528 [details] [diff] [review]
better patch

Shaver, your wish is my command.  Check out these comments:

 * Given pc pointing after a property accessing bytecode, return true if the
 * access is a "object-detecting" in the sense used by web pages, e.g., when
 * checking whether document.all is defined.
static JSBool
Detecting(JSContext *cx, jsbytecode *pc)
    JSScript *script;
    jsbytecode *endpc;
    JSOp op;
    JSAtom *atom;

    script = cx->fp->script;
    for (endpc = script->code + script->length; pc < endpc; pc++) {
	/* General case: a branch or equality op follows the access. */
	op = (JSOp) *pc;
	if (js_CodeSpec[op].format & JOF_DETECTING)
	    return JS_TRUE;

	 * Special case #1: handle (document.all == null).  Don't sweat about
	 * JS1.2's revision of the equality operators here.
	if (op == JSOP_NULL) {
	    if (++pc < endpc)
		return *pc == JSOP_EQ || *pc == JSOP_NE;

	 * Special case #2: handle (document.all == undefined).  Don't worry
	 * about someone redefining undefined, which was added by Edition 3,
	 * so was read/write for backward compatibility.
	if (op == JSOP_NAME) {
	    atom = GET_ATOM(cx, script, pc);
	    if (atom == cx->runtime->atomState.typeAtoms[JSTYPE_VOID] &&
		(pc += js_CodeSpec[op].length) < endpc) {
		op = (JSOp) *pc;
		return op == JSOP_EQ || op == JSOP_NE ||
		       op == JSOP_NEW_EQ || op == JSOP_NEW_NE;

	/* At this point, anything but grouping means we're not detecting. */
	if (op != JSOP_GROUP)
    return JS_FALSE;

Ben said ok for aviary branch.	Fixed there and on trunk.

Attachment #154528 - Flags: approval-aviary+
Closed: 18 years ago
Keywords: fixed-aviary1.0
Resolution: --- → FIXED
Flags: blocking-aviary1.0PR?
This patch doesn't apply to 1.7.5.

Is this something we want for 1.7?
Sorry, forgot to mention and add the keyword -- this is all on the 1.7 branch. 
All the aviary JS engine changes were made to the 1.7 branch at the same time.

$ cvs diff -rAVIARY_1_0_20040515_BRANCH -rMOZILLA_1_7_BRANCH jsobj.c
$ cvs diff -rAVIARY_1_0_20040515_BRANCH -rMOZILLA_1_7_BRANCH jsopcode.tbl

Keywords: fixed1.7
Flags: testcase?
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-253150.js,v  <--  regress-253150.js
Flags: testcase? → testcase+
verified fixed.
You need to log in before you can comment on or make changes to this bug.