Closed Bug 782524 Opened 10 years ago Closed 10 years ago

window.navigator instanceof window.Window throws unexpected error


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

Windows 7
Not set



blocking-basecamp -
Tracking Status
firefox16 + fixed
firefox17 + verified
firefox18 + verified


(Reporter: sebo, Assigned: fabrice)



(Keywords: regression)


(1 file)

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

I seem unable to reproduce on my own build from m-c changeset 22288130fea2.
This is still happening on today's nightly (20120816). navigator DOM object cannot be opened.
Regression range: based on nightlies.

But my debug build seems to not be showing the problem???
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?
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: ? → ---
Priority: P2 → --
blocking-basecamp: --- → ?
Since we only support the API on b2g, we should not expose the interface elsewhere.
OK.  You want to patch, or should I?
Attached patch patchSplinter Review
Attachment #666641 - Flags: review?(bzbarsky)
Comment on attachment 666641 [details] [diff] [review]

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.
Comment on attachment 666641 [details] [diff] [review]

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

[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+
Closed: 10 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.
I can confirm that it's already fixed in Firefox 16.0.2. Thanks for fixing!

This is fixed in FF 17.0.1, but broken again on FF 18b4.
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....
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.

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.
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Filed bug 823173.  But we still need a test here!  I guess we can add it in the other bug...
Verified fixed FF 18 based on comment 28.
You need to log in before you can comment on or make changes to this bug.