Closed
Bug 898874
Opened 11 years ago
Closed 11 years ago
NS_ERROR_FAILURE calling QueryInterface on aSubject from dom-storage2-changed notification
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
WORKSFORME
Tracking | Status | |
---|---|---|
firefox22 | --- | unaffected |
firefox23 | --- | wontfix |
firefox24 | --- | unaffected |
People
(Reporter: mozilla, Unassigned)
References
Details
(Keywords: regression)
Attachments
(1 file)
(deleted),
application/x-xpinstall
|
Details |
User Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20100101 Firefox/17.0 (Beta/Release)
Build ID: 20130509111054
Steps to reproduce:
1. Subscribe to the observer notification dom-storage2-changed
2. Change a storage item
3. In observe(), call sSubject.QueryInterface(Ci.nsIDOMStorageEvent)
Actual results:
Exception is thrown, NS_ERROR_FAILURE
Expected results:
No exception. The subject should be of type nsIDOMStorageEvent. This worked until Firefox 23.
Reporter | ||
Comment 2•11 years ago
|
||
Trivial addon to reproduce the exception.
Flags: needinfo?(mozilla)
Reporter | ||
Comment 3•11 years ago
|
||
Attached an xpi. This is a cut-down addon which adds an observer for dom-storage2-changed, then attempts to access the resulting aSubject as nsIDOMStorageEvent.
To see the exception, install the addon (note that it is just a stripped-out Cookie Controller so it will replace that addon if you already have it) and do something that uses DOM Storage. The Firefox start page should trigger it. If not, go here:
http://www.quirksmode.org/html5/tests/storage.html
In Firefox 22, no error and I am able to access the StorageEvent object. In Firefox 23, exception when calling QueryInterface(nsIDOMStorageEvent).
Attachment #783072 -
Attachment mime type: application/octet-stream → application/x-xpinstall
Could you test with Nightly, I'm not sure I'm able to repro it with your add-on (or maybe th add-on is not compatible with FF25 and the error is not thrown).
http://nightly.mozilla.org/
Of course, I'm able to repro the issue in FF23.
Flags: needinfo?(mozilla)
Reporter | ||
Comment 5•11 years ago
|
||
I can go one better. Even the current Aurora (24a02) seems to work OK. Apparently only FF23 is affected.
Flags: needinfo?(mozilla)
Reporter | ||
Updated•11 years ago
|
Summary: NS_ERROR_FAILURE calling QueryInterface on aSubject from dom2-storage2-changed notification → NS_ERROR_FAILURE calling QueryInterface on aSubject from dom-storage2-changed notification
Version: 17 Branch → 23 Branch
Yes, the issue appeared in Nightly FF24, and it has been backported in Aurora FF23.
Then the issue has been fixed someway in Nightly FF24 but the fix has not been backported. That's why FF23 shows the issue, and not FF24.
STR:
1) Install the attached add-on
2) Open http://www.quirksmode.org/html5/tests/storage.html
3) Click on "setItem()" tests
Result:
Erreur : [Exception... "Failure" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: chrome://cookiecontroller/content/cookieController.jsm :: cookieControllerModule.trackStorage :: line 28" data: no]
Regression range:
good=2013-05-16
bad=2013-05-17
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cc139752bed4&tochange=ea767da526ff
Suspected bug:
Boris Zbarsky — Bug 869195. Make QueryInterface be exposed for both chrome and xbl scopes, not just in chrome. r=bholley,peterv
Working range:
bad=2013-06-19
good=2013-06-20
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d2a7cfa34154&tochange=8ea92aeab783
Maybe fixed by:
Boris Zbarsky — Bug 884109.
Blocks: 869195
Status: UNCONFIRMED → NEW
status-firefox22:
--- → unaffected
status-firefox23:
--- → ?
status-firefox24:
--- → unaffected
Component: Untriaged → DOM
Ever confirmed: true
Keywords: regression
Product: Firefox → Core
Comment 7•11 years ago
|
||
Hrm. I would think if the "make QI be exposed" patch were relevant we'd go from having a QI method to not or back, not end up with a QI that throws....
The fix range has bug 847611, which is what I assume fixed this.
The bad news is that 23 is code-frozen as of a few days ago, so it'll be shipping with this bug. I'll see if I can figure out enough about what's going on to at least suggest a workaround.
Comment 8•11 years ago
|
||
OK, so I can certainly reproduce in a build from rev ea767da526ff.
We end up in mozilla::dom::QueryInterface with the "this" object being a StorageEvent, which is not a WebIDL object at that point.
I think this is what's going on: with bug 869195, QueryInterface is now no longer chromeonly, so it ends up installed via webidl quickstubs on the interfaces it's present on. And webidl quickstubs do check whether to define things conditionally, but of course in this case (for a chrome global) the IsChromeOrXBL check on QueryInterface passes.
We had some provisions to not install QueryInterface on protos which correspond to XPConnect objects, but we did that via not installing it except on concrete interfaces that have all-abstract ancestors.
Except Event is not actually abstract in 23, even though it has xpconnect impls, because basic events are in fact on WebIDL bindings. So we install the WebIDL quickstubs on all XPConnect prototypes of interfaces inheriting from nsIDOMEvent, and in particular StorageEvent.prototype.QueryInterface ends up being the WebIDL thing.
So as for workarounds, I think your best bet is:
Cu.lookupMethod(event, "QueryInterface")(Ci.nsIDOMStorageEvent);
for Firefox 23.... or just removing the line entirely, since I would think it's not needed: the event should have nsIDOMStorageEvent in its classinfo.
Bobby, Olli, I don't know that there's anything to really do about this situation at this point: the only thing with webidl quickstubs is EventTarget, which is marked as abstract already, so this problem shouldn't arise again.
Flags: needinfo?(bugs)
Flags: needinfo?(bobbyholley+bmo)
Comment 9•11 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #8)
> Bobby, Olli, I don't know that there's anything to really do about this
> situation at this point: the only thing with webidl quickstubs is
> EventTarget, which is marked as abstract already, so this problem shouldn't
> arise again.
And that's because of Window, right?
But yeah, SGTM.
Flags: needinfo?(bobbyholley+bmo)
Comment 10•11 years ago
|
||
> And that's because of Window, right?
There might be other event targets too; I haven't checked so far because there's no point worrying about it until Window is on WebIDL.
I guess I'll mark this wontfix for 23 and worksforme otherwise...
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
Comment 11•11 years ago
|
||
Not all event targets are webidl-fied..
But yeah, I think we need to just wontfix this :/ Too late to fix it for 23.
Flags: needinfo?(bugs)
Assignee | ||
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•