[webkit-reviews] review denied: [Bug 16814] Give plugin a chance to handle ActiveX objects : [Attachment 22688] Patch that allow custom build WebKit to handle ActiveX objects differently

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 27 18:07:48 PDT 2008


Eric Seidel <eric at webkit.org> has denied Eric Seidel <eric at webkit.org>'s
request for review:
Bug 16814: Give plugin a chance to handle ActiveX objects
https://bugs.webkit.org/show_bug.cgi?id=16814

Attachment 22688: Patch that allow custom build WebKit to handle ActiveX
objects differently
https://bugs.webkit.org/attachment.cgi?id=22688&action=edit

------- Additional Comments from Eric Seidel <eric at webkit.org>
Another way to do this would be to  have this code ask the platform via
PluginInfoStore.h or a similar abstraction.  That would allow platforms which
do or don't have these plugins to override these mappings at runtime, which is
probably more useful (As then you can test for the presence of the plugin)
instead of using a compile-time check.

Also, "ShouldUseEmbedForObject" is not following WebKit-style naming
conventions.  WebKit style asks that we use camelCase such as:
"shouldUseEmbedForObject()"

Also, we generally try to use early-returns instead of long ifs, which would
mean this function would become:

if (!attributes)
    return true;
for (... ) {
   ...

Again, I think that this is probably better suited as a platform-specific
question, which means we should add a function to PluginInfoStore.h which
answers this question on a per-platform basis.

Looking forward to Anders' thoughts.


More information about the webkit-reviews mailing list