Closed
Bug 889821
Opened 11 years ago
Closed 11 years ago
Custom JS nsIChannel.asyncOpen implementation gets called with invalid parameters when request comes from an HTMLObjectElement
Categories
(Core Graveyard :: Plug-ins, defect)
Tracking
(firefox22 wontfix, firefox23+ verified, firefox24+ verified, firefox25 verified)
VERIFIED
FIXED
mozilla25
People
(Reporter: vincent.cariven, Assigned: johns)
References
Details
(Keywords: regression)
Attachments
(3 files, 2 obsolete files)
7.52 KB,
application/octet-stream
|
Details | |
1.79 KB,
text/html
|
Details | |
3.23 KB,
patch
|
johns
:
review+
bajaj
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
johns
:
checkin+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/27.0.1453.116 Safari/537.36 Steps to reproduce: In an extension define a custom nsIChannel implementation. Make an HTML page with an object tag whose data attribute makes a request to the custom nsIChannel. Actual results: The request fails because onStartRequest method is not defined on the nsIStreamListener passed to the nsIChannel.asyncOpen method. Expected results: The request should not have failed and should have answered back to the object the channel content. Once a request is bound to this channel implementation, its asyncOpen is called with an nsIStreamListener as the first parameter. When the request issuer is an HTMLObjectElement, this first argument does not have the nsIStreamListener methods visible (onStartRequest, onStopRequest) since FF22. Previous FF version worked very well. Requests issued by other means (HTMLIFrameElement, HTMLImageElement, top level window,...) work very well.
To reproduce the wrong behaviour, install this minimimalist extension which defines a custom nsIProtocolHandler (test://) bound a custom nsIChannel implementation(TestChannel)
This test page reproduces the bug. Take a look at the Error Console to view the message raised by the CustomChannel.
Just anothor comment to be more precise : these custom JS channels worked very well before FF 22 (and still do when not requested from an <object> tag !)
![]() |
||
Comment 4•11 years ago
|
||
Fun. This is like bug 883968 but for the argument to asyncOpen. John, do you have time to fix this, or should I? We can reuse the same shim object, I would expect and just have it forward along more interfaces...
Assignee | ||
Comment 5•11 years ago
|
||
Sure I'll take a look
Assignee: nobody → jschoenick
Status: NEW → ASSIGNED
Component: DOM: Core & HTML → Plug-ins
Flags: needinfo?(jschoenick)
OS: Windows 7 → All
Hardware: x86_64 → All
Assignee | ||
Comment 6•11 years ago
|
||
Well the static_cast thing is awful, but this is already a mess for this interface. I guess the alternative would be to add these interfaces to nsIObjectLoadingContent or restore nsIInterfaceRequestor to it, or maybe have the shim have two redundant pointers to the same class. Given that this is file-local this seems like the sanest approach.
Attachment #773411 -
Flags: review?(bzbarsky)
![]() |
||
Comment 7•11 years ago
|
||
Comment on attachment 773411 [details] [diff] [review] Extend interface shim to allow nsIStreamListener callbacks to object/embed/applet tags to be used from script >+ nsCOMPtr<nsIObjectLoadingContent> mContent; How about nsRefPtr<nsObjectLoadingContent> and then you don't need the casts? Also, I think you need nsIRequestObserver here too. And no need to add anything to ObjectInterfaceRequestorShim::GetInterface.
Attachment #773411 -
Flags: review?(bzbarsky) → review-
Assignee | ||
Comment 8•11 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #7) > Comment on attachment 773411 [details] [diff] [review] > Extend interface shim to allow nsIStreamListener callbacks to > object/embed/applet tags to be used from script > > >+ nsCOMPtr<nsIObjectLoadingContent> mContent; > > How about nsRefPtr<nsObjectLoadingContent> and then you don't need the casts? nsRefPtr fails, we discussed this on IRC and decided keeping the static cast was the least-ugly option. Added a comment > Also, I think you need nsIRequestObserver here too. Fixed > And no need to add anything to ObjectInterfaceRequestorShim::GetInterface. Removed
Attachment #773411 -
Attachment is obsolete: true
Attachment #773427 -
Flags: review?(bzbarsky)
![]() |
||
Comment 9•11 years ago
|
||
Comment on attachment 773427 [details] [diff] [review] Extend interface shim to allow nsIStreamListener callbacks to object/embed/applet tags to be used from script No need for the bug comment in the checkin comment here. ;) You need nsIRequestObserver in the QI impl for ObjectInterfaceRequestorShim too. r=me with that
Attachment #773427 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 11•11 years ago
|
||
Comment on attachment 773469 [details] [diff] [review] Extend interface shim to allow nsIStreamListener callbacks to object/embed/applet tags to be used from script. r=bz https://hg.mozilla.org/integration/mozilla-inbound/rev/ddebb5983363
Attachment #773469 -
Flags: review+
Attachment #773469 -
Flags: checkin+
Comment 12•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ddebb5983363
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
![]() |
||
Updated•11 years ago
|
status-firefox22:
--- → wontfix
tracking-firefox23:
--- → ?
tracking-firefox24:
--- → ?
Keywords: regression
Assignee | ||
Comment 13•11 years ago
|
||
Comment on attachment 773469 [details] [diff] [review] Extend interface shim to allow nsIStreamListener callbacks to object/embed/applet tags to be used from script. r=bz [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 827158 User impact if declined: Addons will be unable to intercept certain channel functions for object/embed/applet elements that worked properly in FF21, and attempting to do so may break said element. Testing completed (on m-c, etc.): On m-c, verified this fixes provided testcase. Risk to taking this patch (and alternatives if risky): Low, just shuffles around interfaces to ensure we're exposing an XPIDL object to some calls. String or IDL/UUID changes made by this patch: None
Attachment #773469 -
Flags: approval-mozilla-beta?
Attachment #773469 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
status-firefox24:
--- → affected
Comment 14•11 years ago
|
||
Comment on attachment 773469 [details] [diff] [review] Extend interface shim to allow nsIStreamListener callbacks to object/embed/applet tags to be used from script. r=bz Given the low risk of the patch and this is recent regression on Fx22, approving on aurora
Attachment #773469 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•11 years ago
|
![]() |
||
Comment 16•11 years ago
|
||
Mostly extension compatibility: an extension that implements a protocol handler and channel won't work for <object>.
Updated•11 years ago
|
Comment 17•11 years ago
|
||
Comment on attachment 773469 [details] [diff] [review] Extend interface shim to allow nsIStreamListener callbacks to object/embed/applet tags to be used from script. r=bz In the interest of extension compatibility and given the low risk assessment here, let's take this on beta too.
Attachment #773469 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 18•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/16ee09f2fb7e https://hg.mozilla.org/releases/mozilla-beta/rev/1ae00cd78569
Updated•11 years ago
|
Attachment #770794 -
Attachment mime type: text/plain → text/html
Comment 19•11 years ago
|
||
(In reply to caribou from comment #2) > Created attachment 770794 [details] > Test page to view once the customchannel.xpi is installed > > This test page reproduces the bug. Take a look at the Error Console to view > the message raised by the CustomChannel. Reproduced on FF 22. Verified fixed FF 23b10 Win 7 x64.
Reporter | ||
Comment 20•11 years ago
|
||
Thank you very much to all of you for the rapidity for this bug fix and the quick integration to FF23. Regards, Vincent
Updated•11 years ago
|
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•