259935 - document.all can be easily detected

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Steve Chapel]

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]

Attachment: testcase

This testcase displays "IE" if document.all is detected, and "not IE" if it is
not detected.

Comment 2

[Boris Zbarsky [:bzbarsky]]

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]]

Ok, this is easy to fix.

/be

Comment 4

[Brendan Eich [:brendan]]

Attachment: 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]]

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]]

Fixed everywhere.

/be

Comment 7

[Bob Clary [:bc] (inactive)]

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] (inactive)]

seamonkey trunk: not IE
seamonkey 1.7:   not IE
firefox trunk:   not startable 
firefox branch:  not IE

Comment 9

[Boris Zbarsky [:bzbarsky]]

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

Comment 10

[Bob Clary [:bc] (inactive)]

var f = { ie: document.all };

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

shows IE

Comment 11

[Bob Clary [:bc] (inactive)]

Attachment: testcase 2

Comment 12

[José Jeria]

(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]

(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] (inactive)]

(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]]

Attachment: 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]]

Comment on attachment 161784 [details] [diff] [review]
fix for aviary branch

Imputing r=shaver, and self-approving.

/be

Comment 17

[Brendan Eich [:brendan]]

Fixed, yet again and for good.

/be

Comment 18

[Bob Clary [:bc] (inactive)]

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] (inactive)]

verified fixed.