Last Comment Bug 259935 - document.all can be easily detected
: document.all can be easily detected
Status: VERIFIED FIXED
: fixed-aviary1.0, fixed1.7.5
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: P1 major (vote)
: mozilla1.8alpha4
Assigned To: Brendan Eich [:brendan]
: Hixie (not reading bugmail)
: Jason Orendorff [:jorendorff]
Mentors:
http://geocities.sbc.yahoo.com/
: 260834 (view as bug list)
Depends on:
Blocks: 260685
  Show dependency treegraph
 
Reported: 2004-09-16 18:57 PDT by Steve Chapel
Modified: 2006-08-19 11:02 PDT (History)
9 users (show)
brendan: blocking1.7.5+
brendan: blocking‑aviary1.0+
brendan: blocking1.8a4+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (266 bytes, text/html)
2004-09-16 18:59 PDT, Steve Chapel
no flags Details
proposed fix (2.39 KB, patch)
2004-09-20 16:18 PDT, Brendan Eich [:brendan]
shaver: review+
brendan: approval‑aviary+
brendan: approval1.7.5+
Details | Diff | Splinter Review
testcase 2 (233 bytes, text/html)
2004-10-09 13:34 PDT, Bob Clary [:bc:]
no flags Details
fix for aviary branch (1.79 KB, patch)
2004-10-11 12:46 PDT, Brendan Eich [:brendan]
brendan: review+
brendan: approval‑aviary+
brendan: approval1.7.5+
Details | Diff | Splinter Review

Description Steve Chapel 2004-09-16 18:57:54 PDT
Code such as the following can detect Mozilla's "undetected" document.all:

function foo() {
    this.ie = document.all;
}
var f = new foo();
if (f.ie) // ...

Code like this is used on websites, such as http://geocities.sbc.yahoo.com/ and
http://us.js1.yimg.com/us.yimg.com/lib/g/ylib_dom.js
Comment 1 Steve Chapel 2004-09-16 18:59:32 PDT
Created attachment 159150 [details]
testcase

This testcase displays "IE" if document.all is detected, and "not IE" if it is
not detected.
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2004-09-20 14:42:26 PDT
I think we've seen this code pattern before, but that was a theoretical
discussion; this is mentioning common web sites...

Setting blocking request flags, since this could be pretty serious.  We really
don't want to be commonly flagged as IE.
Comment 3 Brendan Eich [:brendan] 2004-09-20 16:14:23 PDT
Ok, this is easy to fix.

/be
Comment 4 Brendan Eich [:brendan] 2004-09-20 16:18:19 PDT
Created attachment 159519 [details] [diff] [review]
proposed fix

Make JSOP_SETPROP detecting, as JSOP_SETNAME and JSOP_SETVAR are already.  I
made JSOP_SETELEM as well, for consistency, even though it's unlikely anyone
computes "ie" in i and then does 'this[i] = document.all;'.

/be
Comment 5 Brendan Eich [:brendan] 2004-09-20 17:22:12 PDT
Comment on attachment 159519 [details] [diff] [review]
proposed fix

Need this for 1.0, and 1.7. Self-approving.

/be
Comment 6 Brendan Eich [:brendan] 2004-09-20 17:26:34 PDT
Fixed everywhere.

/be
Comment 7 Bob Clary [:bc:] 2004-09-20 18:15:48 PDT
Ok, I have been testing the open TE bugs but have not retested the top 70 sites
in some time. I will run tests on the top 70 sites and the pages linked from
their homepages and will investigate the usage of document.all in terms of if we
are detected as IE anywhere. Since the number of pages to be tested will be
several times more than the open TE list I will not test the mouse over/out
handling, but will just look for usage up to the page's load event. This may
take several days, so don't hold your breath.
Comment 8 Bob Clary [:bc:] 2004-09-20 20:17:25 PDT
seamonkey trunk: not IE
seamonkey 1.7:   not IE
firefox trunk:   not startable 
firefox branch:  not IE
Comment 9 Boris Zbarsky [:bz] (still a bit busy) 2004-09-22 13:10:33 PDT
*** Bug 260834 has been marked as a duplicate of this bug. ***
Comment 10 Bob Clary [:bc:] 2004-10-09 13:33:58 PDT
var f = { ie: document.all };

if (f.ie) {
    document.writeln("IE");
} else {
    document.writeln("not IE");
}

shows IE

Comment 11 Bob Clary [:bc:] 2004-10-09 13:34:59 PDT
Created attachment 161601 [details]
testcase 2
Comment 12 José Jeria 2004-10-09 13:51:37 PDT
(In reply to comment #10)
> var f = { ie: document.all };
> 
> if (f.ie) {
>     document.writeln("IE");
> } else {
>     document.writeln("not IE");
> }
> 
> shows IE

This seems to only happend in quirks mode
> 

Comment 13 Steve Chapel 2004-10-10 19:32:14 PDT
(In reply to comment #10)
> var f = { ie: document.all };
> 
> if (f.ie) {

Are there any pages on the web that actually use code like this to detect IE?
Comment 14 Bob Clary [:bc:] 2004-10-11 11:12:02 PDT
(In reply to comment #13)
> Are there any pages on the web that actually use code like this to detect IE?

Not that I know of, but I haven't look at each use of document.all. I ran across
it by accident and felt it should have been handled similarly to the class-based
property.

Comment 15 Brendan Eich [:brendan] 2004-10-11 12:46:14 PDT
Created attachment 161784 [details] [diff] [review]
fix for aviary branch

I hope this bug won't be reopened again.  I'd rather deal with real-world
problems such as IE's treating language=JavaScript1.2 as if it were
language=JavaScript.

/be
Comment 16 Brendan Eich [:brendan] 2004-10-11 12:47:26 PDT
Comment on attachment 161784 [details] [diff] [review]
fix for aviary branch

Imputing r=shaver, and self-approving.

/be
Comment 17 Brendan Eich [:brendan] 2004-10-11 12:56:51 PDT
Fixed, yet again and for good.

/be
Comment 18 Bob Clary [:bc:] 2006-02-13 18:54:57 PST
Checking in regress-259935.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-259935.js,v  <--  regress-259935.js
initial revision: 1.1
done
Comment 19 Bob Clary [:bc:] 2006-08-19 11:02:14 PDT
verified fixed.

Note You need to log in before you can comment on or make changes to this bug.