Block all plugins smaller than 5x5px
<rdar://problem/28435204>
Created attachment 364578 [details] Patch
Comment on attachment 364578 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364578&action=review > Source/WebCore/ChangeLog:10 > + 5x5px. Other browsers have implemented this for a while, and now s/5x5px/5pxx5px/ :) > Source/WebKitLegacy/mac/ChangeLog:25 > +2019-03-13 Dean Jackson <dino@apple.com> > + > + Block all plugins smaller than 5x5px > + https://bugs.webkit.org/show_bug.cgi?id=195702 > + <rdar://problem/28435204> > + > + Reviewed by NOBODY (OOPS!). > + > + Removed a function that was never being called. > + > + * WebCoreSupport/WebFrameLoaderClient.h: > + * WebCoreSupport/WebFrameLoaderClient.mm: > + (WebFrameLoaderClient::recreatePlugin): Deleted. > + > +2019-03-13 Dean Jackson <dino@apple.com> > + > + Block all plugins smaller than 5x5px > + https://bugs.webkit.org/show_bug.cgi?id=195702 > + > + Reviewed by NOBODY (OOPS!). > + > + * WebCoreSupport/WebFrameLoaderClient.h: > + * WebCoreSupport/WebFrameLoaderClient.mm: > + (WebFrameLoaderClient::recreatePlugin): Deleted. > + Double ChangeLog! > Source/WebCore/platform/LocalizedStrings.cpp:683 > + return WEB_UI_STRING_KEY("Plug-in too small", "Plug-In too small", "Label text to be used when a plug-in was blocked from loading because it was too small"); You have different capitalization of "plug-in" in the first two strings. Is that intentional? > LayoutTests/plugins/small-plugin-blocked.html:34 > +<embed id="plugin3" type="application/x-webkit-test-netscape" width="4" height="4"></embed> Probably want to do one on the boundary, 5px, as well. And maybe one that has a width too small, but a big height or vice versa.
Comment on attachment 364578 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364578&action=review > Source/WebKit/WebProcess/WebPage/WebPage.cpp:893 > +static bool pluginIsSmall(WebCore::HTMLPlugInElement* pluginElement) Can this take a reference?
Comment on attachment 364578 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364578&action=review > Source/WebCore/html/HTMLPlugInElement.h:86 > + WEBCORE_EXPORT bool isTooSmall() const; "too small" sounds subjective. isBelowEnablingSizeThreshold or something? > Source/WebCore/page/Settings.yaml:814 > +blockSmallPluginsEnabled: "block" is a verb so this is a bit hard to read. "smallPluginBlockingEnabled"? blockingOfSmallPluginsEnabled? > Source/WebKit/Shared/WebPreferences.yaml:1250 > +BlockSmallPluginsEnabled: smallPluginBlockingEnabled > Source/WebKit/Shared/WebPreferences.yaml:1254 > + humanReadableDescription: "Stop plugins smaller than a certain threshold from loading." Maybe you can actually say what the threshold is here. > Source/WebKit/WebProcess/WebPage/WebPage.cpp:900 > + return box.contentWidth() <= smallPluginDimensionThreshold && box.contentHeight() <= smallPluginDimensionThreshold; Is this what other browsers do? Should we not block 1x6px plugins? > LayoutTests/plugins/small-plugin-blocked.html:35 > +<embed id="plugin4" type="application/x-webkit-test-netscape" width="6" height="6"></embed> Maybe test 1x6
Comment on attachment 364578 [details] Patch Attachment 364578 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/11494760 New failing tests: plugins/small-plugin-blocked.html
Created attachment 364586 [details] Archive of layout-test-results from ews103 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 364578 [details] Patch Attachment 364578 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/11494915 New failing tests: fast/frames/sandboxed-iframe-navigation-allowed.html fast/frames/sandboxed-iframe-about-blank.html compositing/plugins/small-to-large-composited-plugin.html
Created attachment 364589 [details] Archive of layout-test-results from ews105 for mac-highsierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Committed r242920: <https://trac.webkit.org/changeset/242920>
Comment on attachment 364578 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364578&action=review >> Source/WebCore/ChangeLog:10 >> + 5x5px. Other browsers have implemented this for a while, and now > > s/5x5px/5pxx5px/ :) fixed. >> Source/WebKitLegacy/mac/ChangeLog:25 >> + > > Double ChangeLog! fixed. >> Source/WebCore/html/HTMLPlugInElement.h:86 >> + WEBCORE_EXPORT bool isTooSmall() const; > > "too small" sounds subjective. isBelowEnablingSizeThreshold or something? changed >> Source/WebCore/page/Settings.yaml:814 >> +blockSmallPluginsEnabled: > > "block" is a verb so this is a bit hard to read. "smallPluginBlockingEnabled"? blockingOfSmallPluginsEnabled? changed >> Source/WebCore/platform/LocalizedStrings.cpp:683 >> + return WEB_UI_STRING_KEY("Plug-in too small", "Plug-In too small", "Label text to be used when a plug-in was blocked from loading because it was too small"); > > You have different capitalization of "plug-in" in the first two strings. Is that intentional? fixed. >> Source/WebKit/Shared/WebPreferences.yaml:1250 >> +BlockSmallPluginsEnabled: > > smallPluginBlockingEnabled Changed. >> Source/WebKit/Shared/WebPreferences.yaml:1254 >> + humanReadableDescription: "Stop plugins smaller than a certain threshold from loading." > > Maybe you can actually say what the threshold is here. I didn't because this string is really only used for a tooltip in the internal debug menu. >> Source/WebKit/WebProcess/WebPage/WebPage.cpp:893 >> +static bool pluginIsSmall(WebCore::HTMLPlugInElement* pluginElement) > > Can this take a reference? fixed. >> Source/WebKit/WebProcess/WebPage/WebPage.cpp:900 >> + return box.contentWidth() <= smallPluginDimensionThreshold && box.contentHeight() <= smallPluginDimensionThreshold; > > Is this what other browsers do? Should we not block 1x6px plugins? Other browsers seem to check for any dimension. >> LayoutTests/plugins/small-plugin-blocked.html:35 >> +<embed id="plugin4" type="application/x-webkit-test-netscape" width="6" height="6"></embed> > > Maybe test 1x6 Added tests for 5x5 1x6 and 6x1
This change broke the windows build: win10-release\build\source\webkitlegacy\win\webcoresupport\webframeloaderclient.h(190): error C3668: 'WebFrameLoaderClient::recreatePlugin': method with override specifier 'override' did not override any base class methods https://build.webkit.org/builders/Apple%20Win%2010%20Release%20%28Build%29/builds/2851
(In reply to Ryan Haddad from comment #12) > This change broke the windows build: > win10- > release\build\source\webkitlegacy\win\webcoresupport\webframeloaderclient. > h(190): error C3668: 'WebFrameLoaderClient::recreatePlugin': method with > override specifier 'override' did not override any base class methods > https://build.webkit.org/builders/Apple%20Win%2010%20Release%20%28Build%29/ > builds/2851 Attempted a fix in https://trac.webkit.org/changeset/242925