Bug 108368 – Implement a custom appearance for the snapshotted plugin background

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Dean Jackson]

Turn the blur behind the label background back on.

[Radar WebKit Bug Importer]

<rdar://problem/13117802>

[Simon Fraser (smfr)]

Why was it removed?

[Dean Jackson]

(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]

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]

Created attachment 192089 [details]
Patch

[Dean Jackson]

Comment on attachment 192089 [details]
Patch

temporarily removing r? to see if i can avoid having a custom element

[Dean Jackson]

Created attachment 192123 [details]
Patch

[Tim Horton]

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]

(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]

Created attachment 192130 [details]
Patch for landing

[Dean Jackson]

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 on attachment 192130 [details]
Patch for landing

Attachment 192130 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17054303

[Early Warning System Bot]

Comment on attachment 192130 [details]
Patch for landing

Attachment 192130 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/16990308

[Dean Jackson]

Created attachment 192132 [details]
Another Patch for Landing

[Dean Jackson]

Committed r145196: <http://trac.webkit.org/changeset/145196>