WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
107890
Add a user agent stylesheet for plugins
https://bugs.webkit.org/show_bug.cgi?id=107890
Summary
Add a user agent stylesheet for plugins
Dean Jackson
Reported
2013-01-24 17:57:10 PST
In preparation for a Shadow DOM on plugins, provide a UA stylesheet that can be overridden by the RenderTheme.
Attachments
Patch
(17.71 KB, patch)
2013-01-24 19:11 PST
,
Dean Jackson
no flags
Details
Formatted Diff
Diff
Patch
(18.23 KB, patch)
2013-01-25 09:48 PST
,
Dean Jackson
ap
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2013-01-24 17:57:51 PST
<
rdar://problem/13083415
>
Dean Jackson
Comment 2
2013-01-24 19:11:55 PST
Created
attachment 184640
[details]
Patch
Jon Lee
Comment 3
2013-01-24 19:36:09 PST
Comment on
attachment 184640
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=184640&action=review
An unofficial r=me.
> Source/WebCore/rendering/RenderTheme.h:89 > // adjust the default CSS rules in html.css, quirks.css, or mediaControls.css
Should also mention plugIns.css.
Jon Lee
Comment 4
2013-01-24 19:36:17 PST
Comment on
attachment 184640
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=184640&action=review
An unofficial r=me.
> Source/WebCore/rendering/RenderTheme.h:89 > // adjust the default CSS rules in html.css, quirks.css, or mediaControls.css
Should also mention plugIns.css.
Jon Lee
Comment 5
2013-01-24 19:36:18 PST
Comment on
attachment 184640
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=184640&action=review
An unofficial r=me.
> Source/WebCore/rendering/RenderTheme.h:89 > // adjust the default CSS rules in html.css, quirks.css, or mediaControls.css
Should also mention plugIns.css.
Jon Lee
Comment 6
2013-01-24 19:36:19 PST
Comment on
attachment 184640
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=184640&action=review
An unofficial r=me.
> Source/WebCore/rendering/RenderTheme.h:89 > // adjust the default CSS rules in html.css, quirks.css, or mediaControls.css
Should also mention plugIns.css.
Alexey Proskuryakov
Comment 7
2013-01-24 22:07:59 PST
Comment on
attachment 184640
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=184640&action=review
> Source/WebCore/css/plugIns.css:13 > + * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
APPLE *COMPUTER*?
> Source/WebCore/css/plugIns.css:25 > +/* plug-ins */
Is this helpful?
> Source/WebCore/page/ChromeClient.h:379 > + virtual String plugInStartLabelSubtitle() const { return String(); }; > + virtual String plugInExtraStyleSheet() const { return String(); };
Please remove the trailing semicolons.
> Source/WebCore/rendering/RenderTheme.h:92 > + virtual String extraPlugInsStyleSheet() { return String(); };
Please remove the trailing semicolon.
> Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageUIClient.cpp:198 > + RefPtr<WebString> styleSheet = adoptRef(toImpl(m_client.plugInExtraStyleSheet(m_client.clientInfo)));
This looks suspicious. Does the result of toImpl really need to be adopted? The function does not have "copy" or "create" in the name.
Dean Jackson
Comment 8
2013-01-25 09:41:52 PST
Comment on
attachment 184640
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=184640&action=review
>> Source/WebCore/css/plugIns.css:13 >> + * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY > > APPLE *COMPUTER*?
Fixed.
>> Source/WebCore/css/plugIns.css:25 >> +/* plug-ins */ > > Is this helpful?
Not at the moment, but the other UA CSS files have similar comments at the top. I'll remove it.
>> Source/WebCore/page/ChromeClient.h:379 >> + virtual String plugInExtraStyleSheet() const { return String(); }; > > Please remove the trailing semicolons.
Done.
>>>>> Source/WebCore/rendering/RenderTheme.h:89 >>>>> // adjust the default CSS rules in html.css, quirks.css, or mediaControls.css >>>> >>>> Should also mention plugIns.css. >>> >>> Should also mention plugIns.css. >> >> Should also mention plugIns.css. > > Should also mention plugIns.css.
Done.
>> Source/WebCore/rendering/RenderTheme.h:92 >> + virtual String extraPlugInsStyleSheet() { return String(); }; > > Please remove the trailing semicolon.
Done. C&P from the line below, so I fixed it there too.
>> Source/WebKit2/WebProcess/InjectedBundle/InjectedBundlePageUIClient.cpp:198 >> + RefPtr<WebString> styleSheet = adoptRef(toImpl(m_client.plugInExtraStyleSheet(m_client.clientInfo))); > > This looks suspicious. Does the result of toImpl really need to be adopted? The function does not have "copy" or "create" in the name.
I was confused because the other methods in this class that return Strings adopt the result of toImpl - generateFileForUpload and shouldGenerateFileForUpload, even without "copy" or "create" in the name. I've removed it.
Dean Jackson
Comment 9
2013-01-25 09:48:53 PST
Created
attachment 184766
[details]
Patch
Alexey Proskuryakov
Comment 10
2013-01-25 10:05:49 PST
Comment on
attachment 184766
[details]
Patch
> I was confused because the other methods in this class that return Strings adopt the result of toImpl - generateFileForUpload and shouldGenerateFileForUpload, even without "copy" or "create" in the name. I've removed it.
I don't know which is correct, but it would be easy to tell from checking the code, or stepping through it. Seems likely that there is something to fix here, if only naming.
Dean Jackson
Comment 11
2013-01-25 13:56:28 PST
Committed
r140863
: <
http://trac.webkit.org/changeset/140863
>
Dean Jackson
Comment 12
2013-01-25 13:57:28 PST
Landed with names altered to prepend "create". I forgot to remove the comment in the plugIns.css file, but that will go in the next commit.
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