Bug 75790

Summary: PDFPlugins don't load when plugins are disabled, but they should
Product: WebKit Reporter: Chris Petersen <c.petersen87>
Component: FramesAssignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, beidson, commit-queue, eric.carlson, esprehn+autocc, japhet, ossy, sam, thorton, webkit-ews
Priority: P3 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.7   
Bug Depends on: 115487, 116289    
Bug Blocks:    
Attachments:
Description Flags
preliminary patch
webkit-ews: commit-queue-
changelogs
none
patch andersca: review+

Chris Petersen
Reported 2012-01-07 23:21:34 PST
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 )
Attachments
preliminary patch (34.85 KB, patch)
2013-05-01 02:25 PDT, Tim Horton 		webkit-ews: commit-queue-
DetailsFormatted DiffDiff
changelogs (48.40 KB, patch)
2013-05-01 03:43 PDT, Tim Horton 		no flags
DetailsFormatted DiffDiff
patch (35.92 KB, patch)
2013-05-03 16:38 PDT, Tim Horton 		andersca: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2012-01-08 00:02:26 PST
Looks like a PDF iframe gets (unsuccessfully) handled as a media document when plug-ins are disabled.
Tim Horton
Comment 2 2012-11-29 18:43:27 PST
<rdar://problem/11650197>
Tim Horton
Comment 3 2012-11-29 18:46:26 PST
The symptom has changed a bit, now you get garbage (raw PDF data) in the frame instead.
Tim Horton
Comment 4 2013-05-01 02:25:21 PDT
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 5 2013-05-01 02:30:40 PDT
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 6 2013-05-01 02:31:24 PDT
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 7 2013-05-01 02:58:53 PDT
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
Comment 8 2013-05-01 03:43:11 PDT
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 9 2013-05-01 03:45:22 PDT
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
Comment 10 2013-05-01 03:48:06 PDT
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
Comment 11 2013-05-03 16:38:44 PDT
Created attachment 200507 [details] patch
Anders Carlsson
Comment 12 2013-05-16 14:50:01 PDT
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
Comment 13 2013-05-16 17:45:00 PDT
http://trac.webkit.org/changeset/150227
Csaba Osztrogonác
Comment 14 2013-05-16 22:03:44 PDT
(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
Note You need to log in before you can comment on or make changes to this bug.