WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(20.15 KB, patch)
2013-03-07 18:32 PST
,
Dean Jackson
no flags
Details
Formatted Diff
Diff
Patch for landing
(22.43 KB, patch)
2013-03-07 19:15 PST
,
Dean Jackson
no flags
Details
Formatted Diff
Diff
Another Patch for Landing
(22.44 KB, patch)
2013-03-07 19:32 PST
,
Dean Jackson
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2013-01-30 12:29:13 PST
<
rdar://problem/13117802
>
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
Created
attachment 192089
[details]
Patch
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
Created
attachment 192123
[details]
Patch
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
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 13
2013-03-07 19:21:32 PST
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
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
Committed
r145196
: <
http://trac.webkit.org/changeset/145196
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug