The default bug view has changed. See this FAQ.

window.navigator instanceof window.Window throws unexpected error

VERIFIED FIXED in Firefox 16

Status

()

Core
DOM: Core & HTML
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: sebo, Assigned: fabrice)

Tracking

({regression})

Trunk
mozilla18
x86
Windows 7
regression
Points:
---

Firefox Tracking Flags

(blocking-basecamp:-, firefox16+ fixed, firefox17+ verified, firefox18+ verified)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Executing "window.navigator instanceof window.Window" or checking against other globally defined objects like window.Document or window.Node causes an unexpected error.

This error occurs within the current (2012-08-13) Firefox Nightly and Aurora build. It does not happen in the latest Beta or 14.0.1.

Test case:
1. Open the Web Console
2. Type "window.navigator instanceof window.Window" (without quotes) and hit Enter

Sebastian
I seem unable to reproduce on my own build from m-c changeset 22288130fea2.

Comment 2

5 years ago
This is still happening on today's nightly (20120816). navigator DOM object cannot be opened.
Regression range: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a31fc9052840&tochange=e61399f31505 based on nightlies.

But my debug build seems to not be showing the problem???
Status: UNCONFIRMED → NEW
tracking-firefox17: --- → ?
tracking-firefox18: --- → ?
Ever confirmed: true
Keywords: regression
OK, something is _really_ wacky here.  Today's nightly shows the problem for me, but neither the debug nor opt build I made last night from a clean tree show the problem...

Marking security-sensitive, since my best guess is that something is reading memory it should not somewhere.
Group: core-security
Severity: normal → major
A debug try build says:

###!!! ASSERTION: nsDOMConstructor::HasInstance can't get interface info.: 'Error', file ../../../dom/base/nsDOMClassInfo.cpp, line 6391

so perhaps something is not being packaged that should be?
6459        iim->GetInfoForIID(class_interface, getter_AddRefs(if_info));
6460        if (!if_info) {
6461          NS_ERROR("nsDOMConstructor::HasInstance can't get interface info.");
6462          return NS_ERROR_UNEXPECTED;
6463        }

(gdb) p/x *class_interface
$2 = {
  m0 = 0x91e90dd, 
  m1 = 0xe8b, 
  m2 = 0x463d, 
  m3 = {0x8c, 0xdc, 0x92, 0x25, 0xd3, 0xa6, 0xff, 0x90}
}

That's the IID for nsIDOMNavigatorSystemMessages.  Is the xpt for that not being packaged?  But why is that the class_interface here?
Blocks: 755245

Updated

5 years ago
blocking-basecamp: --- → ?
Oh, because we check all the classinfo interfaces in that loop.

So yeah, it looks like we only package dom_messages.xpt on b2g, but expose the interface everywhere?  That's very broken.

We need to fix this on beta before we ship Fx16 next week...  Not sure whether the fix is to ship the xpt or to not expose the interface; over to Fabrice to decide that.
Assignee: nobody → fabrice
Group: core-security
Severity: major → normal
blocking-basecamp: ? → ---
tracking-firefox16: --- → ?
Priority: P2 → --
blocking-basecamp: --- → ?
(Assignee)

Comment 8

5 years ago
Since we only support the API on b2g, we should not expose the interface elsewhere.
OK.  You want to patch, or should I?
(Assignee)

Comment 10

5 years ago
Created attachment 666641 [details] [diff] [review]
patch
Attachment #666641 - Flags: review?(bzbarsky)
Comment on attachment 666641 [details] [diff] [review]
patch

r=me, especially if you add a mochitest for the original testcase in this bug (just checking that it returns false)... ;)
Attachment #666641 - Flags: review?(bzbarsky) → review+
Sounds like this will be very low risk (assuming there's no IID change with add-on compat impact), but I want to make sure we need to actually make a change here. We haven't heard user complaints related to this, so what's the risk to leaving this in? Long term issue with the availability of this interface, once people start actually using it?

Today (EOD) will be our final beta build for FF16.
The user-facing issue is that some scripts will throw exceptions when they should not.  Whether this breaks sites in practice is an open question, of course.
This seems like something which could easily be hit in the wild and could potentially cause a dot-release if we didn't fix it, and I believe we should take the fix.

Updated

5 years ago
tracking-firefox16: ? → +
tracking-firefox17: ? → +
tracking-firefox18: ? → +
(Assignee)

Comment 15

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d86a9822bd60
(Assignee)

Comment 16

5 years ago
Comment on attachment 666641 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Exposing an interface to classInfo but not providing implementation.
User impact if declined: Some valid code is throwing exceptions without this patch
Risk to taking this patch (and alternatives if risky): No risk. 
String or UUID changes made by this patch: None
Attachment #666641 - Flags: approval-mozilla-beta?
Attachment #666641 - Flags: approval-mozilla-aurora?
Comment on attachment 666641 [details] [diff] [review]
patch

[Triage Comment]
Approving for branches as well. Please land asap to make our last beta build today.
Attachment #666641 - Flags: approval-mozilla-beta?
Attachment #666641 - Flags: approval-mozilla-beta+
Attachment #666641 - Flags: approval-mozilla-aurora?
Attachment #666641 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 18

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/1bdd0bd4dd1d
https://hg.mozilla.org/releases/mozilla-beta/rev/8a8a932fbef4

Updated

5 years ago
status-firefox16: --- → fixed
status-firefox17: --- → fixed
status-firefox18: --- → fixed
https://hg.mozilla.org/mozilla-central/rev/d86a9822bd60
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
This doesn't block basecamp (if it were still open :) ).
blocking-basecamp: ? → -
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0
Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/17.0 Firefox/17.0

Verified the fix on the latest Firefox 17.0 beta builds (beta 6) and the exception doesn't appear anymore.
Marking verified for Firefox 17.
status-firefox17: fixed → verified
(Reporter)

Comment 22

4 years ago
I can confirm that it's already fixed in Firefox 16.0.2. Thanks for fixing!

Sebastian
Status: RESOLVED → VERIFIED
This is fixed in FF 17.0.1, but broken again on FF 18b4.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Er...  What happened to the test I asked for in comment 11?
That said, this worksforme on trunk.  Sounds like a branch-only issue....
(Reporter)

Comment 26

4 years ago
I can confirm the observations.
It for me on FF 17.0.1/19.0a2 (2012-12-18)/20.0a1 (2012-12-19), but is broken in the lastest 18.0 beta.

Sebastian

Updated

4 years ago
status-firefox18: fixed → affected
OK, I can reproduce on beta.  The issue there is nsIDOMNavigatorTime.

Let's get a separate bug filed on that, since people apparently went and regressed that and it's a different issue from this bug, though related.
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago4 years ago
Resolution: --- → FIXED
Filed bug 823173.  But we still need a test here!  I guess we can add it in the other bug...

Updated

4 years ago
status-firefox18: affected → fixed
Verified fixed FF 18 based on comment 28.
Status: RESOLVED → VERIFIED
status-firefox18: fixed → verified
You need to log in before you can comment on or make changes to this bug.