Summary: | [Modern Media Controls] Add a MediaControlsHost API to retrieve the shadow root CSS | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||||||
Component: | Media | Assignee: | Antoine Quint <graouts> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, graouts, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | Safari 10 | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 163500 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Antoine Quint
2016-10-16 06:42:00 PDT
Antti added support for scoped stylesheets in user agent shadow roots with https://bugs.webkit.org/show_bug.cgi?id=163212. Created attachment 291746 [details]
Patch
Comment on attachment 291746 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=291746&action=review > Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp:292 > + if (page) > + return RenderTheme::themeForPage(page)->mediaControlsStyleSheet(); > + return ""; We use early return, so the null page case should come first. Returning "" is an inefficient way to return the empty string. The efficient way is: return emptyString(); > Source/WebCore/css/CSSDefaultStyleSheets.cpp:187 > + mediaRules = emptyString(); Because of the isEmpty check below, this means we will use the media controls user agent style sheet and the extraMediaControlsStyleSheet. Is that really what we want in this case? (In reply to comment #4) > Comment on attachment 291746 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=291746&action=review > > > Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp:292 > > + if (page) > > + return RenderTheme::themeForPage(page)->mediaControlsStyleSheet(); > > + return ""; > > We use early return, so the null page case should come first. Will change when landing patch. > Returning "" is an inefficient way to return the empty string. The efficient > way is: > > return emptyString(); Will change when landing patch. > > Source/WebCore/css/CSSDefaultStyleSheets.cpp:187 > > + mediaRules = emptyString(); > > Because of the isEmpty check below, this means we will use the media > controls user agent style sheet and the extraMediaControlsStyleSheet. Is > that really what we want in this case? extraMediaControlsStyleSheet is only implemented on GTK, and presumably the runtime flag for modern-media-controls will only ever be true for Cocoa, but it would probably be safer to only have extraMediaControlsStyleSheet if the modern-media-controls flag is off. Will change when landing. Created attachment 291844 [details]
Patch for landing
Created attachment 291845 [details]
Patch for landing
Comment on attachment 291845 [details] Patch for landing Clearing flags on attachment: 291845 Committed r207421: <http://trac.webkit.org/changeset/207421> All reviewed patches have been landed. Closing bug. |