Bug 29054 – Remove dead ActiveX code in RenderPartObject

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[John Abd-El-Malek]

There's some code in that file that was added when Chromium was hosting some ActiveX controls.  That's not the case anymore, so that code can be removed from WebKit to clean things up.

[John Abd-El-Malek]

Created attachment 39213 [details]
Proposed patch

[John Abd-El-Malek]

Created attachment 39215 [details]
Proposed patch

Previous patch didn't have an updated ChangeLog.

[Eric Seidel (no email)]

It's difficult for me to tell at first glance if all of this code is only used by Chromium or not.  I guess it would be slightly easier if we referenced the original revision where this all was added, as then I would compare this removal with the additions in the original patch.

[John Abd-El-Malek]

(In reply to comment #3)
> It's difficult for me to tell at first glance if all of this code is only used
> by Chromium or not.  I guess it would be slightly easier if we referenced the
> original revision where this all was added, as then I would compare this
> removal with the additions in the original patch.

Here's the original patch: http://trac.webkit.org/changeset/39115

The code has changed a bit since then through refactoring/cleaning up by others.

Thanks

[Dimitri Glazkov (Google)]

Comment on attachment 39215 [details]
Proposed patch

sounds good.

[Dimitri Glazkov (Google)]

Comment on attachment 39215 [details]
Proposed patch

jam will commit by hand.

[Darin Adler]

I'm pretty sure this patch changed the behavior of Safari on Windows. Was that intentional? Was it correct?

[John Abd-El-Malek]

(In reply to comment #7)
> I'm pretty sure this patch changed the behavior of Safari on Windows. Was that
> intentional? Was it correct?

This would only be the case if Safari provides a generic ActiveX host control, i.e. one that supports the "application/x-oleobject" mime type.  I don't think that's the case?  Chrome used to do that as an experiment in web compat, but it hurt more than it helped, so we removed it.

[Darin Adler]

(In reply to comment #8)
> This would only be the case if Safari provides a generic ActiveX host control,
> i.e. one that supports the "application/x-oleobject" mime type.  I don't think
> that's the case?  Chrome used to do that as an experiment in web compat, but it
> hurt more than it helped, so we removed it.

Of course Safari does not provide such a thing, but I had the impression that it could be a plug-in that provides it, not Safari itself.

I guess I didn't realize this was a Chrome-specific experiment. If so, then I think the ifdefs were done wrong, because the hook was included in all WebKit browsers, not just Chrome. I guess it was included in the non-Chrome versions of WebKit for no good reason, and there's a good chance nobody tried to use this hook on other platforms, so it's probably OK to take it out.

[John Abd-El-Malek]

(In reply to comment #9)
> (In reply to comment #8)
> > This would only be the case if Safari provides a generic ActiveX host control,
> > i.e. one that supports the "application/x-oleobject" mime type.  I don't think
> > that's the case?  Chrome used to do that as an experiment in web compat, but it
> > hurt more than it helped, so we removed it.
> 
> Of course Safari does not provide such a thing, but I had the impression that
> it could be a plug-in that provides it, not Safari itself.
> 
> I guess I didn't realize this was a Chrome-specific experiment. If so, then I
> think the ifdefs were done wrong, because the hook was included in all WebKit
> browsers, not just Chrome. I guess it was included in the non-Chrome versions
> of WebKit for no good reason, and there's a good chance nobody tried to use
> this hook on other platforms, so it's probably OK to take it out.

I'm not aware of any NPAPI plugin that registers for that mime type, since it's really a generic type used for ActiveX.  I'm quite certain no other WebKit port is crazy enough to try this, but if there are any regressions we can revisit this.

[John Abd-El-Malek]

Committed in r48274.