Attachment 357172 Details for Bug 192638 – Patch

[patch] Patch

bug-192638-20181212150526.patch (text/plain), 11.81 KB, created by Per Arne Vollan on 2018-12-12 15:05:27 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Per Arne Vollan

Created: 2018-12-12 15:05:27 PST

Size: 11.81 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 239129)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,29 @@
>+2018-12-12 Per Arne Vollan <pvollan@apple.com>
>+
>+ [macOS] Inline WebVTT styles should override styles from Captions settings in System Preferences
>+ https://bugs.webkit.org/show_bug.cgi?id=192638
>+
>+ Reviewed by NOBODY (OOPS!).
>+
>+ It is currently not possible to override caption styles generated from System Preferences with inline
>+ WebVTT styles without adding !important. The reason for this is that the generated styles from
>+ System preferences are author styles which have higher priority than the inline WebVTT styles, which
>+ are user styles in the video user agent shadow tree. This can be fixed by moving the generated styles
>+ to the video user agent shadow tree. Inline WebVTT styles will then have higher priority since they
>+ are added after the generated styles. This patch also fixes a problem where inline styles could be
>+ added twice to the video user agent shadow root.
>+
>+ Test: media/track/track-cue-css.html
>+
>+ * dom/ExtensionStyleSheets.cpp:
>+ (WebCore::ExtensionStyleSheets::updateInjectedStyleSheetCache const):
>+ * html/track/VTTCue.cpp:
>+ (WebCore::VTTCue::getDisplayTree):
>+ * page/CaptionUserPreferences.cpp:
>+ (WebCore::CaptionUserPreferences::setCaptionsStyleSheetOverride):
>+ * page/Page.cpp:
>+ (WebCore::Page::setCaptionUserPreferencesStyleSheet):
>+
> 2018-12-11 Ryosuke Niwa <rniwa@webkit.org>
> 
> Make HTMLConverter take two Positions in preparation to make it work with shadow DOM
>Index: Source/WebCore/dom/ExtensionStyleSheets.cpp
>===================================================================
>--- Source/WebCore/dom/ExtensionStyleSheets.cpp	(revision 239014)
>+++ Source/WebCore/dom/ExtensionStyleSheets.cpp	(working copy)
>@@ -137,15 +137,6 @@ void ExtensionStyleSheets::updateInjecte
> else
> m_injectedAuthorStyleSheets.append(WTFMove(sheet));
> });
>- 
>- if (!owningPage->captionUserPreferencesStyleSheet().isEmpty()) {
>- // Identify our override style sheet with a unique URL - a new scheme and a UUID.
>- static NeverDestroyed<URL> captionsStyleSheetURL(URL(), "user-captions-override:01F6AF12-C3B0-4F70-AF5E-A3E00234DC23");
>-
>- auto sheet = createExtensionsStyleSheet(const_cast<Document&>(m_document), captionsStyleSheetURL, owningPage->captionUserPreferencesStyleSheet(), UserStyleAuthorLevel);
>-
>- m_injectedAuthorStyleSheets.append(WTFMove(sheet));
>- }
> }
> 
> void ExtensionStyleSheets::invalidateInjectedStyleSheetCache()
>Index: Source/WebCore/html/track/VTTCue.cpp
>===================================================================
>--- Source/WebCore/html/track/VTTCue.cpp	(revision 239014)
>+++ Source/WebCore/html/track/VTTCue.cpp	(working copy)
>@@ -894,12 +894,19 @@ VTTCueBox& VTTCue::getDisplayTree(const 
> displayTree->setFontSizeFromCaptionUserPrefs(fontSize);
> displayTree->applyCSSProperties(videoSize);
> 
>+ if (displayTree->document().page()) {
>+ auto cssString = displayTree->document().page()->captionUserPreferencesStyleSheet();
>+ auto style = HTMLStyleElement::create(HTMLNames::styleTag, displayTree->document(), false);
>+ style->setTextContent(cssString);
>+ displayTree->appendChild(style);
>+ }
>+
> const auto& styleSheets = track()->styleSheets();
> if (styleSheets) {
> for (const auto& cssString : *styleSheets) {
>- auto style = HTMLStyleElement::create(HTMLNames::styleTag, m_cueBackdropBox->document(), false);
>+ auto style = HTMLStyleElement::create(HTMLNames::styleTag, displayTree->document(), false);
> style->setTextContent(cssString);
>- m_cueBackdropBox->appendChild(style);
>+ displayTree->appendChild(style);
> }
> }
> 
>Index: Source/WebCore/page/CaptionUserPreferences.cpp
>===================================================================
>--- Source/WebCore/page/CaptionUserPreferences.cpp	(revision 239014)
>+++ Source/WebCore/page/CaptionUserPreferences.cpp	(working copy)
>@@ -314,8 +314,13 @@ int CaptionUserPreferences::textTrackLan
> 
> void CaptionUserPreferences::setCaptionsStyleSheetOverride(const String& override)
> {
>+ if (override == m_captionsStyleSheetOverride)
>+ return;
>+
> m_captionsStyleSheetOverride = override;
> updateCaptionStyleSheetOverride();
>+ if (!m_timer.isActive())
>+ m_timer.startOneShot(0_s);
> }
> 
> void CaptionUserPreferences::updateCaptionStyleSheetOverride()
>Index: Source/WebCore/page/Page.cpp
>===================================================================
>--- Source/WebCore/page/Page.cpp	(revision 239014)
>+++ Source/WebCore/page/Page.cpp	(working copy)
>@@ -2564,8 +2564,6 @@ void Page::setCaptionUserPreferencesStyl
> return;
> 
> m_captionUserPreferencesStyleSheet = styleSheet;
>- 
>- invalidateInjectedStyleSheetCacheInAllFrames();
> }
> 
> void Page::accessibilitySettingsDidChange()
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 239014)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,16 @@
>+2018-12-12 Per Arne Vollan <pvollan@apple.com>
>+
>+ [macOS] Inline WebVTT styles should override styles from Captions settings in System Preferences
>+ https://bugs.webkit.org/show_bug.cgi?id=192638
>+
>+ Reviewed by NOBODY (OOPS!).
>+
>+ * media/track/captions-webvtt/css-styling.vtt:
>+ * media/track/captions-webvtt/no-css-styling.vtt:
>+ * media/track/track-css-user-override-expected.txt:
>+ * media/track/track-css-user-override.html:
>+ * media/track/track-cue-css-expected.html:
>+
> 2018-12-08 Simon Fraser <simon.fraser@apple.com>
> 
> Allow control over child order when adding nodes to the scrolling tree
>Index: LayoutTests/media/track/track-css-user-override-expected.txt
>===================================================================
>--- LayoutTests/media/track/track-css-user-override-expected.txt	(revision 239014)
>+++ LayoutTests/media/track/track-css-user-override-expected.txt	(working copy)
>@@ -1,6 +1,6 @@
> 
> 
>-internals.captionsStyleSheetOverride = function captionsStyleSheetOverride() { [native code] }
>+internals.captionsStyleSheetOverride = 
> Test that style to all cues is applied correctly.
> EVENT(canplaythrough)
> EVENT(seeked)
>@@ -16,7 +16,7 @@ EXPECTED (getComputedStyle(textTrackDisp
> 
> ** Add an override, without '!important' **
> RUN(internals.setCaptionsStyleSheetOverride('video::cue { color: blue; background-color: yellow; }'))
>-internals.captionsStyleSheetOverride = function captionsStyleSheetOverride() { [native code] }
>+internals.captionsStyleSheetOverride = video::cue { color: blue; background-color: yellow; }
> EXPECTED (getComputedStyle(textTrackDisplayElement(video, 'cue')).color == 'rgb(128, 0, 128)') OK
> EXPECTED (getComputedStyle(textTrackDisplayElement(video, 'cue')).backgroundColor == 'rgb(0, 255, 0)') OK
> 
>Index: LayoutTests/media/track/track-css-user-override.html
>===================================================================
>--- LayoutTests/media/track/track-css-user-override.html	(revision 239014)
>+++ LayoutTests/media/track/track-css-user-override.html	(working copy)
>@@ -30,30 +30,32 @@
> consoleWrite(" ** Add an override, without '!important' **");
> document.body.offsetTop;
> run("internals.setCaptionsStyleSheetOverride('video::cue { color: blue; background-color: yellow; }')");
>- consoleWrite("internals.captionsStyleSheetOverride = " + internals.captionsStyleSheetOverride);
>+ consoleWrite("internals.captionsStyleSheetOverride = " + internals.captionsStyleSheetOverride());
> document.body.offsetTop;
> testExpected("getComputedStyle(textTrackDisplayElement(video, 'cue')).color", "rgb(128, 0, 128)");
> testExpected("getComputedStyle(textTrackDisplayElement(video, 'cue')).backgroundColor", "rgb(0, 255, 0)");
> 
> consoleWrite(" ** Mark background-color '!important' **");
> run("internals.setCaptionsStyleSheetOverride('video::cue { color: blue ; background-color: yellow !important; }')");
>- document.body.offsetTop;
>- testExpected("getComputedStyle(textTrackDisplayElement(video, 'cue')).color", "rgb(128, 0, 128)");
>- testExpected("getComputedStyle(textTrackDisplayElement(video, 'cue')).backgroundColor", "rgb(255, 255, 0)");
>-
>- consoleWrite(" ** Mark color '!important' **");
>- run("stylesheet.insertRule('video::cue { color: blue !important; background-color: yellow !important; }', 0)");
>- document.body.offsetTop;
>- testExpected("getComputedStyle(textTrackDisplayElement(video, 'cue')).color", "rgb(0, 0, 255)");
>- testExpected("getComputedStyle(textTrackDisplayElement(video, 'cue')).backgroundColor", "rgb(255, 255, 0)");
>-
>- consoleWrite("");
>- endTest();
>+ // The style is not immediately updated, schedule the rest of the test with a zero timeout timer.
>+ setTimeout(function() {
>+ testExpected("getComputedStyle(textTrackDisplayElement(video, 'cue')).color", "rgb(128, 0, 128)");
>+ testExpected("getComputedStyle(textTrackDisplayElement(video, 'cue')).backgroundColor", "rgb(255, 255, 0)");
>+
>+ consoleWrite(" ** Mark color '!important' **");
>+ run("stylesheet.insertRule('video::cue { color: blue !important; background-color: yellow !important; }', 0)");
>+ document.body.offsetTop;
>+ testExpected("getComputedStyle(textTrackDisplayElement(video, 'cue')).color", "rgb(0, 0, 255)");
>+ testExpected("getComputedStyle(textTrackDisplayElement(video, 'cue')).backgroundColor", "rgb(255, 255, 0)");
>+
>+ consoleWrite("");
>+ endTest();
>+ }, 100);
> }
> 
> function loaded()
> {
>-consoleWrite(" internals.captionsStyleSheetOverride = " + internals.captionsStyleSheetOverride);
>+ consoleWrite(" internals.captionsStyleSheetOverride = " + internals.captionsStyleSheetOverride());
> 
> consoleWrite("Test that style to all cues is applied correctly.");
> findMediaElement();
>Index: LayoutTests/media/track/track-cue-css-expected.html
>===================================================================
>--- LayoutTests/media/track/track-cue-css-expected.html	(revision 239014)
>+++ LayoutTests/media/track/track-cue-css-expected.html	(working copy)
>@@ -14,6 +14,10 @@
> color: red;
> font-size: 15px;
> }
>+ video::cue {
>+ color: green;
>+ font-size: 15px;
>+ }
> </style>
> <script>
> function seeked()
>Index: LayoutTests/media/track/captions-webvtt/css-styling.vtt
>===================================================================
>--- LayoutTests/media/track/captions-webvtt/css-styling.vtt	(revision 239014)
>+++ LayoutTests/media/track/captions-webvtt/css-styling.vtt	(working copy)
>@@ -11,6 +11,12 @@ color: red;
> font-size: 15px;
> }
> 
>+STYLE
>+video::cue {
>+color: green;
>+font-size: 15px;
>+}
>+
> hello
> 00:00:00.000 --> 00:00:10.000
>-Hello.
>+Hello first cue.
>Index: LayoutTests/media/track/captions-webvtt/no-css-styling.vtt
>===================================================================
>--- LayoutTests/media/track/captions-webvtt/no-css-styling.vtt	(revision 239014)
>+++ LayoutTests/media/track/captions-webvtt/no-css-styling.vtt	(working copy)
>@@ -2,4 +2,4 @@ WEBVTT
> 
> hello
> 00:00:00.000 --> 00:00:10.000
>-Hello.
>+Hello first cue.

Actions: View | Formatted Diff | Diff

Attachments on bug 192638: 357165 | 357172 | 357190