Bug 186578 – Supported plugin check should be based on plugin identifier

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[youenn fablet]

Detect WebKit built-in PDF plug-in using a bundle identifier

[youenn fablet]

<rdar://problem/40523828>

[youenn fablet]

Created attachment 342608 [details]
Patch

[Darin Adler]

Comment on attachment 342608 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342608&action=review

> Source/WebCore/plugins/PluginData.cpp:57
> +    return plugin.bundleIdentifier == "com.apple.webkit.builtInPDFPlugin";

Seems a little unsafe to have this be a case sensitive check since bundle identifiers are not case sensitive. Could avoid the case sensitivity by using equalLettersIgnoringASCIICase or equalIgnoringASCIICase.

Would also be nice to not risk this getting out of sync if we change the identifier in the future for some reason, but I suppose it’s not that terrible to have it here.

[Zach Li]

Comment on attachment 342608 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=342608&action=review

> Source/WebKit/UIProcess/Plugins/PluginInfoStore.h:68
>      void addSupportedPlugin(String&& matchingDomain, String&& name, HashSet<String>&& mimeTypes, HashSet<String> extensions);

This should be changed to `identifier` from `name`.

[youenn fablet]

Created attachment 342696 [details]
Patch for landing

[youenn fablet]

Thanks for the reviews.

> > Source/WebCore/plugins/PluginData.cpp:57
> > +    return plugin.bundleIdentifier == "com.apple.webkit.builtInPDFPlugin";
> 
> Seems a little unsafe to have this be a case sensitive check since bundle
> identifiers are not case sensitive. Could avoid the case sensitivity by
> using equalLettersIgnoringASCIICase or equalIgnoringASCIICase.

Done

> Would also be nice to not risk this getting out of sync if we change the
> identifier in the future for some reason, but I suppose it’s not that
> terrible to have it here.

Right, I also thought it was not great but not too bad either.

> > Source/WebKit/UIProcess/Plugins/PluginInfoStore.h:68
> >      void addSupportedPlugin(String&& matchingDomain, String&& name, HashSet<String>&& mimeTypes, HashSet<String> extensions);
> 
> This should be changed to `identifier` from `name`.

Done

[WebKit Commit Bot]

Comment on attachment 342696 [details]
Patch for landing

Clearing flags on attachment: 342696

Committed r232815: <https://trac.webkit.org/changeset/232815>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.