Closed Bug 842799 Opened 10 years ago Closed 10 years ago

|Document.prototype instanceof Node| inside <script> returns false


(Core :: DOM: Core & HTML, defect)

21 Branch
Windows 7
Not set



Tracking Status
firefox21 + fixed
firefox22 + verified
firefox23 --- verified
firefox24 --- verified


(Reporter: alice0775, Assigned: peterv)




(Keywords: regression)


(3 files)

Attached file testcase
Build Identifier:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130219 Firefox/21.0 ID:20130219031055

Zlip792 reported the problem in .

alert(Document.prototype instanceof Node);

This happens when running the code in <script>.
This also happens when evaluate the code in Error Console.
However, this problem does not happen when evaluate the code in the Scratchpad and Web Console.

Steps to Reproduce:
1. Open testcase

Actual Results:

Expected  Results:

Regression window(m-c)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130215 Firefox/21.0 ID:20130215133452
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130216 Firefox/21.0 ID:20130216064952

Regression window(m-i)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130215 Firefox/21.0 ID:20130215141251
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130215 Firefox/21.0 ID:20130215143355

Triggered by:
  9a12a0f8c8be	Peter Van der Beken — Fix for bug 838269 (Support cross-global |... instanceof DOMInterface|). r=bz.
So... This was a purposeful change.  If we do want this to return true that can be done, I guess.  Otherwise we should ask Microsoft to fix their test.

Cameron, thoughts?
So this is testing if an interface prototype object is instanceof the instance object?  Web IDL has never required that to be true, AFAICR.  Despite the fact that built-ins like Number, Date, etc. do behave like this, I think I remember TC39 folks saying that it's not a great pattern to follow.  I think we should ask for the test to be fixed.

I have no idea why this being inside a <script> makes a difference here.  The weirdness that the Web Console does with the global might be confusing matters.
No, this is testing whether an interface prototype object for interface A is instanceof the interface object for interface B where A inherits from B.

I too have no idea what's different in the web console case; presumably some aspect of the sandbox is making this "work" somehow.
Given we do need to fix this:

A -> Node
O -> Node.prototype
V -> [[Prototype]] of Document.prototype, which is Node.prototype

O and V refer to the same object.

Attachment 710305 [details] [diff] probably didn't suffer from this flaw.
Oh, hmm.  So that still has the "walk up the proto chain" bit, I guess?  <sigh>.

Well, that's easy enough to add...
Attached patch v1Splinter Review
Assignee: nobody → peterv
Attachment #716159 - Flags: review?(bzbarsky)
Comment on attachment 716159 [details] [diff] [review]

> +        js::GetObjectClass(proto) == interfacePrototypeClasp) {

Why do we want this bit?  The spec just says to check for object identity, no?

But also, how does this deal with cross-compartment __proto__ links?  Seems like we should be comparing "proto" to the reflection of interfacePrototype into the compartment of instance or something.

r=me with those fixed.
Attachment #716159 - Flags: review?(bzbarsky) → review+
Does this affect users at all or just running a test case? Not clear why this would be a release blocker.
It can affect website compat, and it's a correctness issue that shows up on commonly run correctness tests.

Given that we have a patch, I believe we should just make sure we actually get that patch landed.  If by some miracle we get to release day and that hasn't happened, I wouldn't block the release on this, probably, given the info we have so far.
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Attached patch v1Splinter Review
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 838269
User impact if declined: broken sites
Testing completed (on m-c, etc.): landed on m-c, includes a test
Risk to taking this patch (and alternatives if risky): low-risk
String or UUID changes made by this patch: none
Attachment #722186 - Flags: review+
Attachment #722186 - Flags: approval-mozilla-aurora?
Comment on attachment 722186 [details] [diff] [review]

low risk fix which may help with web compat issues .Patched is well baked on m-c, approving for uplift
Attachment #722186 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed on Windows 7, 64 bits, with Firefox 22 beta 2, Aurora 23.0 and latest Nightly 24.0:

- User agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:22.0) Gecko/20100101 Firefox/22.0, Build ID: 20130521223249

- User agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20130526 Firefox/23.0, Build ID: 20130526004017

- User agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:24.0) Gecko/20130525 Firefox/24.0, Build ID: 20130525062525
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.