[webkit-reviews] review requested: [Bug 16814] Give plugin a chance to handle ActiveX objects : [Attachment 24892] patch v3

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Nov 4 12:34:58 PST 2008


Peter Kasting <pkasting at google.com> has asked Anders Carlsson
<andersca at apple.com> for review:
Bug 16814: Give plugin a chance to handle ActiveX objects
https://bugs.webkit.org/show_bug.cgi?id=16814

Attachment 24892: patch v3
https://bugs.webkit.org/attachment.cgi?id=24892&action=edit

------- Additional Comments from Peter Kasting <pkasting at google.com>
Attempting to resurrect this patch since Rui is busy.

Preliminary warning: I don't know anything about OBJECT or EMBED tags, or this
code, so I'm liable to do stupid things.

This makes the following changes over the previous patch:
* Uses a HashMap instead of a series of conditionals.
* Checks isMIMETypeSupported() before adding a mapping from ActiveX->NPAPI.
* Simplifies and expands logic on when not to proceed into nested EMBEDs.  I
think the previous logic was too specific.  I explained my rationale in a
comment.
* Removes the addition of the "!embed" check to the call to
mapClassIdToServiceType().  I am not sure this is correct.  My logic is, the
only cases where this makes a difference are OBJECT tags with classIds that
don't map to a well-known plugin, that have nested EMBEDs.  This seems rare and
when it does happen I feel like we ought to be able to get the service type
from the object.  However, Rui told me privately that he's seen cases in the
wild of code that has an outer OBJECT with a RealAudio classId, containing a
nested EMBED with a .wma.  If the RealAudio plugin doesn't handle .wma, then
Rui's patch would probably work better than mine.  But I wonder if there's
anything consistent that makes sense when authors do this kind of thing.

This patch has not been tested (at all).  I'm not precisely sure how to write a
layout test for it, but I haven't even smoketested things.  Sample URLs of
pages in the wild affected by this code would probably be useful?


More information about the webkit-reviews mailing list