document.all can be easily detected

VERIFIED FIXED in mozilla1.8alpha4

Status

()

Core
JavaScript Engine
P1
major
VERIFIED FIXED
13 years ago
11 years ago

People

(Reporter: Steve Chapel, Assigned: brendan)

Tracking

({fixed-aviary1.0, fixed1.7.5})

Trunk
mozilla1.8alpha4
fixed-aviary1.0, fixed1.7.5
Points:
---
Bug Flags:
blocking1.7.5 +
blocking-aviary1.0 +
blocking1.8a4 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments)

(Reporter)

Description

13 years ago
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
(Reporter)

Comment 1

13 years ago
Created attachment 159150 [details]
testcase

This testcase displays "IE" if document.all is detected, and "not IE" if it is
not detected.
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.
Flags: blocking1.8a4?
Flags: blocking1.7.x?
Flags: blocking-aviary1.0?
(Assignee)

Comment 3

13 years ago
Ok, this is easy to fix.

/be
Assignee: general → brendan
Component: DOM: Level 0 → JavaScript Engine
Flags: blocking1.8a4?
Flags: blocking1.8a4+
Flags: blocking1.7.x?
Flags: blocking1.7.x+
Flags: blocking-aviary1.0?
Flags: blocking-aviary1.0+
Priority: -- → P1
Target Milestone: --- → mozilla1.8alpha4
(Assignee)

Comment 4

13 years ago
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
(Assignee)

Updated

13 years ago
Attachment #159519 - Flags: review?(shaver)
Attachment #159519 - Flags: review?(shaver) → review+
(Assignee)

Comment 5

13 years ago
Comment on attachment 159519 [details] [diff] [review]
proposed fix

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

/be
Attachment #159519 - Flags: approval1.7.x+
Attachment #159519 - Flags: approval-aviary+
(Assignee)

Comment 6

13 years ago
Fixed everywhere.

/be
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Keywords: fixed-aviary1.0, fixed1.7.x
Resolution: --- → FIXED

Comment 7

13 years ago
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

13 years ago
seamonkey trunk: not IE
seamonkey 1.7:   not IE
firefox trunk:   not startable 
firefox branch:  not IE

Updated

13 years ago
Blocks: 260685
*** Bug 260834 has been marked as a duplicate of this bug. ***

Comment 10

13 years ago
var f = { ie: document.all };

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

shows IE

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 11

13 years ago
Created attachment 161601 [details]
testcase 2

Comment 12

13 years ago
(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
> 

(Reporter)

Comment 13

13 years ago
(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

13 years ago
(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.

(Assignee)

Comment 15

13 years ago
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
(Assignee)

Comment 16

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

Imputing r=shaver, and self-approving.

/be
Attachment #161784 - Flags: review+
Attachment #161784 - Flags: approval1.7.x+
Attachment #161784 - Flags: approval-aviary+
(Assignee)

Comment 17

13 years ago
Fixed, yet again and for good.

/be
Status: REOPENED → RESOLVED
Last Resolved: 13 years ago13 years ago
Resolution: --- → FIXED

Updated

12 years ago
Flags: testcase?

Comment 18

12 years ago
Checking in regress-259935.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-259935.js,v  <--  regress-259935.js
initial revision: 1.1
done
Flags: testcase? → testcase+

Comment 19

11 years ago
verified fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.