| Summary: | Supported plugin check should be based on plugin identifier | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||
| Component: | WebCore Misc. | Assignee: | youenn fablet <youennf> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | a.tion.surf, cdumez, commit-queue, darin, dbates, ews-watchlist, ggaren, japhet, jberlin, rmondello, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
youenn fablet
2018-06-12 16:11:53 PDT
Created attachment 342608 [details]
Patch
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. 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`. Created attachment 342696 [details]
Patch for landing
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 Comment on attachment 342696 [details] Patch for landing Clearing flags on attachment: 342696 Committed r232815: <https://trac.webkit.org/changeset/232815> All reviewed patches have been landed. Closing bug. |