Commit 76f1eb03 authored by Abigail Klein's avatar Abigail Klein Committed by Commit Bot

[webVTT] Remove condition that overrides all author text track styles

When text track styles were first being introduced, it was decided that any time user styles are present, all author styles for the text track will be ignored (discussed in https://codereview.chromium.org/921833003). There was a complaint by a customer about this policy, and we can also observe that Safari does not follow. For this reason, we are undoing this decision in this patch.

Bug: 979649
Change-Id: I944eeb75d765e7899850ef65337786a6913c59d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1733928Reviewed-by: default avatarAlan Cutter <alancutter@chromium.org>
Reviewed-by: default avatarMounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Abigail Klein <abigailbklein@google.com>
Cr-Commit-Position: refs/heads/master@{#688082}
parent 2e969864
......@@ -1446,23 +1446,6 @@ static inline bool IsValidFirstLetterStyleProperty(CSSPropertyID id) {
}
}
static bool ShouldIgnoreTextTrackAuthorStyle(const Document& document) {
Settings* settings = document.GetSettings();
if (!settings)
return false;
// Ignore author specified settings for text tracks when any of the user
// settings are present.
if (!settings->GetTextTrackBackgroundColor().IsEmpty() ||
!settings->GetTextTrackFontFamily().IsEmpty() ||
!settings->GetTextTrackFontStyle().IsEmpty() ||
!settings->GetTextTrackFontVariant().IsEmpty() ||
!settings->GetTextTrackTextColor().IsEmpty() ||
!settings->GetTextTrackTextShadow().IsEmpty() ||
!settings->GetTextTrackTextSize().IsEmpty())
return true;
return false;
}
static bool PassesPropertyFilter(ValidPropertyFilter valid_property_filter,
CSSPropertyID property,
const Document& document) {
......@@ -1472,8 +1455,7 @@ static bool PassesPropertyFilter(ValidPropertyFilter valid_property_filter,
case ValidPropertyFilter::kFirstLetter:
return IsValidFirstLetterStyleProperty(property);
case ValidPropertyFilter::kCue:
return IsValidCueStyleProperty(property) &&
!ShouldIgnoreTextTrackAuthorStyle(document);
return IsValidCueStyleProperty(property);
}
NOTREACHED();
return true;
......
<!DOCTYPE html>
<title>Test that WebVTT objects are being styled correctly based on user settings that should override author settings.</title>
<title>Test that WebVTT objects are being styled correctly based on author settings that should override user settings.</title>
<script src="../media-controls.js"></script>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
......@@ -11,8 +11,6 @@ video::cue(c) {
text-shadow: 3px 3px #00ff00;
font-size: 20px;
font-family: arial;
font-style: normal;
font-variant: normal;
}
</style>
<video>
......@@ -31,10 +29,8 @@ async_test(function(t) {
assert_equals(cueStyle.textShadow, "rgb(0, 255, 0) 3px 3px 0px");
assert_equals(cueStyle.fontSize, "20px");
assert_equals(cueStyle.fontFamily, "arial");
assert_equals(cueStyle.fontStyle, "normal");
assert_equals(cueStyle.fontVariant, "normal");
// Apply user settings and verify they override author-specified settings
// Apply user settings and verify they are overriden by author-specified settings, when the author settings exist.
internals.settings.setTextTrackTextColor("cyan");
internals.settings.setTextTrackBackgroundColor("green");
internals.settings.setTextTrackTextShadow("2px 2px #ff0000")
......@@ -47,13 +43,13 @@ async_test(function(t) {
cue = textTrackCueElementByIndex(video, 0).firstChild.firstElementChild;
cueStyle = getComputedStyle(cue);
assert_equals(cueStyle.color, "rgb(0, 255, 255)");
assert_equals(cueStyle.color, "rgb(255, 0, 0)");
assert_equals(cueStyle.backgroundColor, "rgb(0, 128, 0)");
assert_equals(cueStyle.textShadow, "rgb(255, 0, 0) 2px 2px 0px");
assert_equals(cueStyle.fontSize, "14px");
assert_equals(cueStyle.fontFamily, "fantasy");
assert_equals(cueStyle.textShadow, "rgb(0, 255, 0) 3px 3px 0px");
assert_equals(cueStyle.fontSize, "20px");
assert_equals(cueStyle.fontFamily, "arial");
assert_equals(cueStyle.fontStyle, "italic");
assert_equals(cueStyle.fontVariant, "small-caps");
});
});
</script>
\ No newline at end of file
</script>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment