PDFPlugins don't load when plugins are disabled, but they should

• Component: Frames
• Status: RESOLVED FIXED

[Chris Petersen]

Inline PDF doesn't load when plugins are disabled, need better error message. Maybe by design but inline PDF don't load when plugins are disabled. Right now you see a message Loading... ( like you do when a MP3 as a <audio> element ). However, this message isn't very information.

1) Launch Webkit NB r104385 and disable Plugins in Safari 5.1.1
2) Go to http://www.cs.tut.fi/~jkorpela/html/iframe-pdf.html
3) Notice "Loading.." message appears in both iframes and PDF fails to appear ( No message why )

[Alexey Proskuryakov]

Looks like a PDF iframe gets (unsuccessfully) handled as a media document when plug-ins are disabled.

[Tim Horton]

<rdar://problem/11650197>

[Tim Horton]

The symptom has changed a bit, now you get garbage (raw PDF data) in the frame instead.

[Tim Horton]

Created attachment 200208 [details]
preliminary patch

Rather surprisingly large patch that's not ready for review (no change log, for starters), but if someone takes offense with the way I'm doing this, feel free to let me know!

[Early Warning System Bot]

Comment on attachment 200208 [details]
preliminary patch

Attachment 200208 [details] did not pass qt-ews (qt):
Output: http://webkit-queues.appspot.com/results/356004

[Early Warning System Bot]

Comment on attachment 200208 [details]
preliminary patch

Attachment 200208 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/301065

[Build Bot]

Comment on attachment 200208 [details]
preliminary patch

Attachment 200208 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/271029

[Tim Horton]

Created attachment 200211 [details]
changelogs

I still think it should be possible to do this better without complicating all of the WebKit* side call sites, by making PluginData and the plugin cache invalidate when the plugin pref is flipped, and then just depending on PluginData not including any non-app-plug-ins if the pref is off. However, I'm not sure that'll work since it looks like the FrameLoaderClient can override allowPlugins for arbitrary reasons, not just the state of the setting... so I'm not doing that for now. Let's see what others think.

Not sure I'm totally happy with stuff yet, but this could use a round of scathing review.

[Tim Horton]

Comment on attachment 200211 [details]
changelogs

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

> Source/WebKit2/UIProcess/WebPageProxy.cpp:939
> +    if (MIMETypeRegistry::isPDFOrPostScriptMIMEType(mimeType) && !WebContext::omitPDFSupport())

FWIW I think it's OK that we dropped the case-folding here, as MIMETypeRegistry::canShowMIMEType above is also case-sensitive.

[Tim Horton]

Probably this should be split into more patches:

1) MIMETypeRegistry changes and adoption in WK2.
2) PluginData isApplicationPlugin field and adoption in plugin enumerators.
3) PluginData always being created and findPlugin/createPlugin/etc. getting application-plug-in filters.

[Tim Horton]

Created attachment 200507 [details]
patch

[Anders Carlsson]

Comment on attachment 200507 [details]
patch

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

> Source/WebKit2/UIProcess/Plugins/PluginInfoStore.cpp:125
> +PluginModuleInfo PluginInfoStore::findPluginForMIMEType(const String& mimeType, bool onlyApplicationPlugins) const

Couldn’t this take a AllowedPluginTypes enum instead of a boolean?

> Source/WebKit2/UIProcess/Plugins/PluginInfoStore.cpp:145
> +PluginModuleInfo PluginInfoStore::findPluginForExtension(const String& extension, String& mimeType, bool onlyApplicationPlugins) const

Ditto.

> Source/WebKit2/UIProcess/Plugins/PluginInfoStore.cpp:208
> +PluginModuleInfo PluginInfoStore::findPlugin(String& mimeType, const KURL& url, bool onlyApplicationPlugins)

Ditto.

[Tim Horton]

http://trac.webkit.org/changeset/150227

[Csaba Osztrogonác]

(In reply to comment #13)
> http://trac.webkit.org/changeset/150227

FYI: It caused a serious regression - https://bugs.webkit.org/show_bug.cgi?id=116289