RESOLVED FIXED 108368
Implement a custom appearance for the snapshotted plugin background
https://bugs.webkit.org/show_bug.cgi?id=108368
Summary Implement a custom appearance for the snapshotted plugin background
Dean Jackson
Reported 2013-01-30 12:28:48 PST
Turn the blur behind the label background back on.
Attachments
Patch (17.39 KB, patch)
2013-03-07 15:02 PST, Dean Jackson
no flags
Patch (20.15 KB, patch)
2013-03-07 18:32 PST, Dean Jackson
no flags
Patch for landing (22.43 KB, patch)
2013-03-07 19:15 PST, Dean Jackson
no flags
Another Patch for Landing (22.44 KB, patch)
2013-03-07 19:32 PST, Dean Jackson
no flags
Radar WebKit Bug Importer
Comment 1 2013-01-30 12:29:13 PST
Simon Fraser (smfr)
Comment 2 2013-02-01 10:58:41 PST
Why was it removed?
Dean Jackson
Comment 3 2013-02-01 11:40:08 PST
(In reply to comment #2) > Why was it removed? Moving to a shadow tree solution meant I needed to query the dimensions/location of the label element in order to know where to blur. Previously we always drew the label in the same place. I figured it was better to do this in a separate patch.
Dean Jackson
Comment 4 2013-03-07 14:44:57 PST
Retitling from "Reinstate blur for snapshotted plugin label background" to "Implement a custom renderer for the snapshotted plugin background". Why? Because we shouldn't hardcode an effect into WebCore. By having a custom element in the shadow tree, we can allow ports to do anything they want to the element via their injected CSS. This could be a filter effect, like blur, or an animation, etc. The custom renderer should paint the same as the snapshotted plugin, allowing the content to position the label where ever they like, and have it seamlessly blend into the element content.
Dean Jackson
Comment 5 2013-03-07 15:02:12 PST
Dean Jackson
Comment 6 2013-03-07 15:35:42 PST
Comment on attachment 192089 [details] Patch temporarily removing r? to see if i can avoid having a custom element
Dean Jackson
Comment 7 2013-03-07 18:32:53 PST
Tim Horton
Comment 8 2013-03-07 18:46:49 PST
Comment on attachment 192123 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192123&action=review Seems reasonable to me! Would really like more tests. > Source/WebCore/rendering/RenderThemeMacShared.mm:1717 > + LayoutUnit cWidth = renderBlock->contentWidth(); Leading c looks so weird. > Source/WebCore/rendering/RenderThemeMacShared.mm:1729 > + IntRect alignedRect = pixelSnappedIntRect(rect); Is pixel-snapping right here? > Source/WebCore/rendering/RenderThemeMacShared.mm:1740 > + HTMLElement* plugInOverlay = static_cast<HTMLElement*>(renderBlock->node()); Please use toHTMLElement. > Source/WebCore/rendering/RenderThemeMacShared.mm:1742 > + while (parent && !parent->isPluginElement()) Weird that this isn’t capitalized the same as everything else (but obviously not your fault). > Source/WebCore/rendering/RenderThemeMacShared.mm:1748 > + HTMLPlugInElement* plugInElement = static_cast<HTMLPlugInElement*>(parent); Ditto (if there is one?). > Source/WebCore/rendering/RenderThemeMacShared.mm:1752 > + HTMLPlugInImageElement* plugInImageElement = static_cast<HTMLPlugInImageElement*>(plugInElement); Ditto (if there is one?). > Source/WebCore/rendering/RenderThemeMacShared.mm:1764 > + RenderBox* renderBox = static_cast<RenderBox*>(renderBlock); Ditto. > Source/WebCore/rendering/RenderThemeMacShared.mm:1770 > + renderBox = static_cast<RenderBox*>(renderBox->parentBox()); Ditto.
Dean Jackson
Comment 9 2013-03-07 18:56:54 PST
(In reply to comment #8) > (From update of attachment 192123 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=192123&action=review > > Seems reasonable to me! Would really like more tests. > > > Source/WebCore/rendering/RenderThemeMacShared.mm:1717 > > + LayoutUnit cWidth = renderBlock->contentWidth(); > > Leading c looks so weird. Fixed. > > > Source/WebCore/rendering/RenderThemeMacShared.mm:1729 > > + IntRect alignedRect = pixelSnappedIntRect(rect); > > Is pixel-snapping right here? > > > Source/WebCore/rendering/RenderThemeMacShared.mm:1740 > > + HTMLElement* plugInOverlay = static_cast<HTMLElement*>(renderBlock->node()); > > Please use toHTMLElement. Fixed. > > > Source/WebCore/rendering/RenderThemeMacShared.mm:1742 > > + while (parent && !parent->isPluginElement()) > > Weird that this isn’t capitalized the same as everything else (but obviously not your fault). Yes :( > > > Source/WebCore/rendering/RenderThemeMacShared.mm:1748 > > + HTMLPlugInElement* plugInElement = static_cast<HTMLPlugInElement*>(parent); > > Ditto (if there is one?). I made one. > > > Source/WebCore/rendering/RenderThemeMacShared.mm:1752 > > + HTMLPlugInImageElement* plugInImageElement = static_cast<HTMLPlugInImageElement*>(plugInElement); > > Ditto (if there is one?). I made one. > > > Source/WebCore/rendering/RenderThemeMacShared.mm:1764 > > + RenderBox* renderBox = static_cast<RenderBox*>(renderBlock); > > Ditto. Done. > > > Source/WebCore/rendering/RenderThemeMacShared.mm:1770 > > + renderBox = static_cast<RenderBox*>(renderBox->parentBox()); In fact, that one isn't even necessary. > > Ditto.
Dean Jackson
Comment 10 2013-03-07 19:15:01 PST
Created attachment 192130 [details] Patch for landing
Dean Jackson
Comment 11 2013-03-07 19:18:24 PST
Eric, I hear you are looking at static_casts and toBlah() methods. I added some to HTMLPlugInElement and HTMLPlugInImageElement here.
Early Warning System Bot
Comment 12 2013-03-07 19:21:17 PST
Early Warning System Bot
Comment 13 2013-03-07 19:21:32 PST
Dean Jackson
Comment 14 2013-03-07 19:32:44 PST
Created attachment 192132 [details] Another Patch for Landing
Dean Jackson
Comment 15 2013-03-08 02:08:51 PST
Note You need to log in before you can comment on or make changes to this bug.