Closed
Bug 767639
Opened 12 years ago
Closed 12 years ago
Remove remaining ActiveX stub
Categories
(Core Graveyard :: Plug-ins, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: johns, Assigned: johns)
References
Details
Attachments
(2 files)
(deleted),
patch
|
jaas
:
review+
johns
:
checkin+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jaas
:
review-
|
Details | Diff | Splinter Review |
nsObjectLoadingContent has code related to trying to load an ActiveX plugin if we have classid="clsid:blah" and a helper-plugin that can actually support activeX. This should just go away as we've not had such a helper plugin in a long time
Assignee | ||
Comment 1•12 years ago
|
||
Assignee | ||
Comment 2•12 years ago
|
||
Comment on attachment 649797 [details] [diff] [review]
Remove activeX stubs and simpify classid/java handling
This simplifies handling java a bit, and removes the special cases for classid now that classid can only be 'java:' URIs
In particular, objects like:
> <object type="application/x-java-vm" data="my.class"></object>
Wont spuriously open data streams that will never be used and possibly generate meaningless errors
Attachment #649797 -
Flags: review?(joshmoz)
Attachment #649797 -
Flags: review?(joshmoz) → review+
Assignee | ||
Comment 3•12 years ago
|
||
Comment on attachment 649797 [details] [diff] [review]
Remove activeX stubs and simpify classid/java handling
https://hg.mozilla.org/integration/mozilla-inbound/rev/471f34ddcceb
try:
https://tbpl.mozilla.org/?tree=Try&rev=02f602af452c
Attachment #649797 -
Flags: checkin+
Comment 4•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Comment 5•12 years ago
|
||
(In reply to John Schoenick [:johns] from comment #0)
> This should just go away as we've not had such a helper plugin in a long time
We re-used this functionality in Wine. We have MSHTML implementation based on our forked (but close to upstream) Gecko fork and our own pseudo-plugin that, in combination with our MSHTML, adds support for ActiveX components. All we need is Gecko to recognize classid="clsid:..." as application/x-oleobject MIME. Thus, the patch from this bug causes big regression for us. We may deal with it in our fork, but it's always nicer to not have downstream changes. And given that we need very little from Gecko, the maintenance cost of having such code is pretty low for Mozilla.
Would you please consider r+ for a patch reverting relevant parts of the patch? I will take care of that if you'd agree.
Assignee | ||
Comment 6•12 years ago
|
||
(In reply to Jacek Caban from comment #5)
> (In reply to John Schoenick [:johns] from comment #0)
> > This should just go away as we've not had such a helper plugin in a long time
>
> We re-used this functionality in Wine. We have MSHTML implementation based
> on our forked (but close to upstream) Gecko fork and our own pseudo-plugin
> that, in combination with our MSHTML, adds support for ActiveX components.
> All we need is Gecko to recognize classid="clsid:..." as
> application/x-oleobject MIME. Thus, the patch from this bug causes big
> regression for us. We may deal with it in our fork, but it's always nicer to
> not have downstream changes. And given that we need very little from Gecko,
> the maintenance cost of having such code is pretty low for Mozilla.
>
> Would you please consider r+ for a patch reverting relevant parts of the
> patch? I will take care of that if you'd agree.
The code necessary to support this is pretty minimal, so it comes down to whether or not we want this hook present. If it's serving a useful purpose somewhere I have no issue with it, so I'll leave it up to josh
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 7•12 years ago
|
||
This restores the activex hook while maintaining the java/uri-handling cleanup I snuck into the patch.
ActiveX similarly uses 'codebase' as its actual-URI, so it can just piggyback on the java logic here
Attachment #655132 -
Flags: review?(joshmoz)
Comment on attachment 655132 [details] [diff] [review]
Restore activex classid hook for object tags
Review of attachment 655132 [details] [diff] [review]:
-----------------------------------------------------------------
I have a strong desire to be helpful here but maintaining code we don't need in a notoriously touchy area for the sake of one particular project that already maintains a fork of Gecko doesn't seem like the right move. I don't want to take on this added compatibility concern for Wine when Wine already has the code (John's patch here is probably good) and a delivery mechanism in place. If the maintenance cost for us is low, then it is probably also low for Wine.
I don't expect this code to change a lot after John's project to clean it up is over in a week or so, and I'd be happy to help answer questions you guys might have in the future.
Attachment #655132 -
Flags: review?(joshmoz) → review-
Assignee | ||
Comment 9•12 years ago
|
||
Remarking as fixed, in that case. I did push this patch to try though, to make sure it doesn't introduce any issues:
https://tbpl.mozilla.org/?tree=Try&rev=fbe4d149be3f
Feel free to ping josh or myself if issues with this arise
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Comment 10•12 years ago
|
||
Thanks a lot for the patch and feedback! It looks good on try and passes all Wine tests, so I've committed it to our repository.
Target Milestone: mozilla17 → ---
Version: Trunk → Other Branch
Updated•3 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•