From cdfee89181b4d52a99c4375bac7b26f76b51da98 Mon Sep 17 00:00:00 2001 From: ibaker Date: Mon, 13 Jul 2020 10:02:25 +0100 Subject: [PATCH] Use CSS inheritance for background-color in WebViewSubtitleOutput CSS background-color isn't inherited to inner HTML elements by default: https://developer.mozilla.org/en-US/docs/Web/CSS/background-color But Android Span styling assumes an outer BackgroundColorSpan will affect inner spans. This usually doesn't make a difference, because HTML elements are transparent by default, so there's an implicit inheritance by just being able to see through to the 'outer' element underneath. However this doesn't work if the inner element sits outside the bounding box of the outer element, e.g. (ruby text, sits above/below) or a with font-size > 100%. END_PUBLIC Demo of and font-size problems: http://go/cpl/ruby-backgrounds/1 Demo of CSS inheritance: http://go/cpl/css-inheritance/1 PiperOrigin-RevId: 320915999 --- .../android/exoplayer2/ui/HtmlUtils.java | 8 + .../exoplayer2/ui/SpannedToHtmlConverter.java | 53 +++++-- .../exoplayer2/ui/WebViewSubtitleOutput.java | 40 ++++- .../ui/SpannedToHtmlConverterTest.java | 143 ++++++++++++------ 4 files changed, 179 insertions(+), 65 deletions(-) diff --git a/library/ui/src/main/java/com/google/android/exoplayer2/ui/HtmlUtils.java b/library/ui/src/main/java/com/google/android/exoplayer2/ui/HtmlUtils.java index 9e3ae65eee..13a14d0033 100644 --- a/library/ui/src/main/java/com/google/android/exoplayer2/ui/HtmlUtils.java +++ b/library/ui/src/main/java/com/google/android/exoplayer2/ui/HtmlUtils.java @@ -32,4 +32,12 @@ import com.google.android.exoplayer2.util.Util; "rgba(%d,%d,%d,%.3f)", Color.red(color), Color.green(color), Color.blue(color), Color.alpha(color) / 255.0); } + + /** + * Returns a CSS selector that selects all elements with {@code class=className} and all their + * descendants. + */ + public static String cssAllClassDescendantsSelector(String className) { + return "." + className + ",." + className + " *"; + } } diff --git a/library/ui/src/main/java/com/google/android/exoplayer2/ui/SpannedToHtmlConverter.java b/library/ui/src/main/java/com/google/android/exoplayer2/ui/SpannedToHtmlConverter.java index 4faab3107b..7ea2b55cf4 100644 --- a/library/ui/src/main/java/com/google/android/exoplayer2/ui/SpannedToHtmlConverter.java +++ b/library/ui/src/main/java/com/google/android/exoplayer2/ui/SpannedToHtmlConverter.java @@ -33,10 +33,15 @@ import com.google.android.exoplayer2.text.span.HorizontalTextInVerticalContextSp import com.google.android.exoplayer2.text.span.RubySpan; import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Util; +import com.google.common.collect.ImmutableMap; import java.util.ArrayList; import java.util.Collections; import java.util.Comparator; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.regex.Pattern; /** @@ -46,7 +51,6 @@ import java.util.regex.Pattern; *

Supports all of the spans used by ExoPlayer's subtitle decoders, including custom ones found * in {@link com.google.android.exoplayer2.text.span}. */ -// TODO: Add support for more span types - only a small selection are currently implemented. /* package */ final class SpannedToHtmlConverter { // Matches /n and /r/n in ampersand-encoding (returned from Html.escapeHtml). @@ -74,16 +78,29 @@ import java.util.regex.Pattern; * @param displayDensity The screen density of the device. WebView treats 1 CSS px as one Android * dp, so to convert size values from Android px to CSS px we need to know the screen density. */ - public static String convert(@Nullable CharSequence text, float displayDensity) { + public static HtmlAndCss convert(@Nullable CharSequence text, float displayDensity) { if (text == null) { - return ""; + return new HtmlAndCss("", /* cssRuleSets= */ ImmutableMap.of()); } if (!(text instanceof Spanned)) { - return escapeHtml(text); + return new HtmlAndCss(escapeHtml(text), /* cssRuleSets= */ ImmutableMap.of()); } Spanned spanned = (Spanned) text; - SparseArray spanTransitions = findSpanTransitions(spanned, displayDensity); + // Use CSS inheritance to ensure BackgroundColorSpans affect all inner elements + Set backgroundColors = new HashSet<>(); + for (BackgroundColorSpan backgroundColorSpan : + spanned.getSpans(0, spanned.length(), BackgroundColorSpan.class)) { + backgroundColors.add(backgroundColorSpan.getBackgroundColor()); + } + HashMap cssRuleSets = new HashMap<>(); + for (int backgroundColor : backgroundColors) { + cssRuleSets.put( + HtmlUtils.cssAllClassDescendantsSelector("bg_" + backgroundColor), + Util.formatInvariant("background-color:%s;", HtmlUtils.toCssRgba(backgroundColor))); + } + + SparseArray spanTransitions = findSpanTransitions(spanned, displayDensity); StringBuilder html = new StringBuilder(spanned.length()); int previousTransition = 0; for (int i = 0; i < spanTransitions.size(); i++) { @@ -104,7 +121,7 @@ import java.util.regex.Pattern; html.append(escapeHtml(spanned.subSequence(previousTransition, spanned.length()))); - return html.toString(); + return new HtmlAndCss(html.toString(), cssRuleSets); } private static SparseArray findSpanTransitions( @@ -137,9 +154,7 @@ import java.util.regex.Pattern; "", HtmlUtils.toCssRgba(colorSpan.getForegroundColor())); } else if (span instanceof BackgroundColorSpan) { BackgroundColorSpan colorSpan = (BackgroundColorSpan) span; - return Util.formatInvariant( - "", - HtmlUtils.toCssRgba(colorSpan.getBackgroundColor())); + return Util.formatInvariant("", colorSpan.getBackgroundColor()); } else if (span instanceof HorizontalTextInVerticalContextSpan) { return ""; } else if (span instanceof AbsoluteSizeSpan) { @@ -231,6 +246,26 @@ import java.util.regex.Pattern; return NEWLINE_PATTERN.matcher(escaped).replaceAll("
"); } + /** Container class for an HTML string and associated CSS rulesets. */ + public static class HtmlAndCss { + + /** A raw HTML string. */ + public final String html; + + /** + * CSS rulesets used to style {@link #html}. + * + *

Each key is a CSS selector, and each value is a CSS declaration (i.e. a semi-colon + * separated list of colon-separated key-value pairs, e.g "prop1:val1;prop2:val2;"). + */ + public final Map cssRuleSets; + + private HtmlAndCss(String html, Map cssRuleSets) { + this.html = html; + this.cssRuleSets = cssRuleSets; + } + } + private static final class SpanInfo { /** * Sort by end index (descending), then by opening tag and then closing tag (both ascending, for diff --git a/library/ui/src/main/java/com/google/android/exoplayer2/ui/WebViewSubtitleOutput.java b/library/ui/src/main/java/com/google/android/exoplayer2/ui/WebViewSubtitleOutput.java index 835a4df43a..f3de4298a5 100644 --- a/library/ui/src/main/java/com/google/android/exoplayer2/ui/WebViewSubtitleOutput.java +++ b/library/ui/src/main/java/com/google/android/exoplayer2/ui/WebViewSubtitleOutput.java @@ -31,11 +31,14 @@ import android.widget.FrameLayout; import androidx.annotation.Nullable; import com.google.android.exoplayer2.text.CaptionStyleCompat; import com.google.android.exoplayer2.text.Cue; +import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Util; import com.google.common.base.Charsets; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; /** * A {@link SubtitleView.Output} that uses a {@link WebView} to render subtitles. @@ -51,6 +54,8 @@ import java.util.List; */ private static final float CSS_LINE_HEIGHT = 1.2f; + private static final String DEFAULT_BACKGROUND_CSS_CLASS = "default_bg"; + /** * A {@link CanvasSubtitleOutput} used for displaying bitmap cues. * @@ -162,7 +167,7 @@ import java.util.List; StringBuilder html = new StringBuilder(); html.append( Util.formatInvariant( - "

cssRuleSets = new HashMap<>(); + cssRuleSets.put( + HtmlUtils.cssAllClassDescendantsSelector(DEFAULT_BACKGROUND_CSS_CLASS), + Util.formatInvariant("background-color:%s;", HtmlUtils.toCssRgba(style.backgroundColor))); for (int i = 0; i < textCues.size(); i++) { Cue cue = textCues.get(i); float positionPercent = (cue.position != Cue.DIMEN_UNSET) ? (cue.position * 100) : 50; @@ -255,6 +262,18 @@ import java.util.List; verticalTranslatePercent = lineAnchorTranslatePercent; } + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert( + cue.text, getContext().getResources().getDisplayMetrics().density); + for (String cssSelector : cssRuleSets.keySet()) { + @Nullable + String previousCssDeclarationBlock = + cssRuleSets.put(cssSelector, cssRuleSets.get(cssSelector)); + Assertions.checkState( + previousCssDeclarationBlock == null + || previousCssDeclarationBlock.equals(cssRuleSets.get(cssSelector))); + } + html.append( Util.formatInvariant( "
", backgroundColorCss)) - .append( - SpannedToHtmlConverter.convert( - cue.text, getContext().getResources().getDisplayMetrics().density)) + .append(Util.formatInvariant("", DEFAULT_BACKGROUND_CSS_CLASS)) + .append(htmlAndCss.html) .append("") .append("
"); } - html.append("
"); + StringBuilder htmlHead = new StringBuilder(); + htmlHead.append(""); + html.insert(0, htmlHead.toString()); + webView.loadData( Base64.encodeToString(html.toString().getBytes(Charsets.UTF_8), Base64.NO_PADDING), "text/html", diff --git a/library/ui/src/test/java/com/google/android/exoplayer2/ui/SpannedToHtmlConverterTest.java b/library/ui/src/test/java/com/google/android/exoplayer2/ui/SpannedToHtmlConverterTest.java index f595d4233b..b9eb6d8e6a 100644 --- a/library/ui/src/test/java/com/google/android/exoplayer2/ui/SpannedToHtmlConverterTest.java +++ b/library/ui/src/test/java/com/google/android/exoplayer2/ui/SpannedToHtmlConverterTest.java @@ -58,27 +58,33 @@ public class SpannedToHtmlConverterTest { "String with colored".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html) + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html) .isEqualTo("String with colored section"); } @Test public void convert_supportsBackgroundColorSpan() { SpannableString spanned = new SpannableString("String with highlighted section"); + int color = Color.argb(51, 64, 32, 16); spanned.setSpan( - new BackgroundColorSpan(Color.argb(51, 64, 32, 16)), + new BackgroundColorSpan(color), "String with ".length(), "String with highlighted".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html) - .isEqualTo( - "String with highlighted" - + " section"); + // Double check the color int is being used for the class name as we expect. + assertThat(color).isEqualTo(859840528); + assertThat(htmlAndCss.cssRuleSets) + .containsExactly(".bg_859840528,.bg_859840528 *", "background-color:rgba(64,32,16,0.200);"); + assertThat(htmlAndCss.html) + .isEqualTo("String with highlighted" + " section"); } @Test @@ -90,9 +96,11 @@ public class SpannedToHtmlConverterTest { "Vertical text with 123".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html) + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html) .isEqualTo( "Vertical text with 123 " + "horizontal numbers"); @@ -109,11 +117,14 @@ public class SpannedToHtmlConverterTest { "String with 10px".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); // 10 Android px are converted to 5 CSS px because WebView treats 1 CSS px as 1 Android dp // and we're using screen density xhdpi i.e. density=2. - assertThat(html).isEqualTo("String with 10px section"); + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html) + .isEqualTo("String with 10px section"); } // Set the screen density so we see that px are handled differently to dp. @@ -127,9 +138,12 @@ public class SpannedToHtmlConverterTest { "String with 10dp".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html).isEqualTo("String with 10dp section"); + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html) + .isEqualTo("String with 10dp section"); } @Test @@ -141,9 +155,12 @@ public class SpannedToHtmlConverterTest { "String with 10%".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html).isEqualTo("String with 10% section"); + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html) + .isEqualTo("String with 10% section"); } @Test @@ -155,9 +172,11 @@ public class SpannedToHtmlConverterTest { "String with Times New Roman".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html) + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html) .isEqualTo( "String with Times New Roman" + " section"); @@ -172,9 +191,11 @@ public class SpannedToHtmlConverterTest { "String with unstyled".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html).isEqualTo("String with unstyled section"); + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html).isEqualTo("String with unstyled section"); } @Test @@ -186,9 +207,11 @@ public class SpannedToHtmlConverterTest { "String with crossed-out".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html) + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html) .isEqualTo( "String with crossed-out section"); } @@ -213,9 +236,11 @@ public class SpannedToHtmlConverterTest { "String with bold, italic and bold-italic".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html) + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html) .isEqualTo( "String with bold, italic and bold-italic sections."); } @@ -235,9 +260,11 @@ public class SpannedToHtmlConverterTest { "String with over-annotated and under-annotated".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html) + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html) .isEqualTo( "String with " + "" @@ -261,33 +288,39 @@ public class SpannedToHtmlConverterTest { "String with underlined".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html).isEqualTo("String with underlined section."); + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html).isEqualTo("String with underlined section."); } @Test public void convert_escapesHtmlInUnspannedString() { - String html = SpannedToHtmlConverter.convert("String with bold tags", displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert("String with bold tags", displayDensity); - assertThat(html).isEqualTo("String with <b>bold</b> tags"); + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html).isEqualTo("String with <b>bold</b> tags"); } @Test public void convert_handlesLinebreakInUnspannedString() { - String html = + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = SpannedToHtmlConverter.convert( "String with\nnew line and\r\ncrlf style too", displayDensity); - assertThat(html).isEqualTo("String with
new line and
crlf style too"); + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html).isEqualTo("String with
new line and
crlf style too"); } @Test public void convert_doesntConvertAmpersandLineFeedToBrTag() { - String html = + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = SpannedToHtmlConverter.convert("String with new line ampersand code", displayDensity); - assertThat(html).isEqualTo("String with&#10;new line ampersand code"); + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html).isEqualTo("String with&#10;new line ampersand code"); } @Test @@ -299,27 +332,32 @@ public class SpannedToHtmlConverterTest { "String with unrecognised".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html).isEqualTo("String with <foo>unrecognised</foo> tags"); + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html) + .isEqualTo("String with <foo>unrecognised</foo> tags"); } @Test public void convert_handlesLinebreakInSpannedString() { - String html = + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = SpannedToHtmlConverter.convert( "String with\nnew line and\r\ncrlf style too", displayDensity); - assertThat(html).isEqualTo("String with
new line and
crlf style too"); + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html).isEqualTo("String with
new line and
crlf style too"); } @Test public void convert_convertsNonAsciiCharactersToAmpersandCodes() { - String html = + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = SpannedToHtmlConverter.convert( new SpannableString("Strìng with 優しいの non-ASCII characters"), displayDensity); - assertThat(html) + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html) .isEqualTo("Strìng with 優しいの non-ASCII characters"); } @@ -337,9 +375,11 @@ public class SpannedToHtmlConverterTest { "String with unrecognised".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html).isEqualTo("String with unrecognised span"); + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html).isEqualTo("String with unrecognised span"); } @Test @@ -351,9 +391,12 @@ public class SpannedToHtmlConverterTest { spanned.setSpan(new StyleSpan(Typeface.ITALIC), start, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); spanned.setSpan(new UnderlineSpan(), start, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html).isEqualTo("String with italic-bold-underlined section"); + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html) + .isEqualTo("String with italic-bold-underlined section"); } @Test @@ -368,9 +411,11 @@ public class SpannedToHtmlConverterTest { "String with italic and bold".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html).isEqualTo("String with italic and bold section"); + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html).isEqualTo("String with italic and bold section"); } @Test @@ -387,8 +432,10 @@ public class SpannedToHtmlConverterTest { "String with italic and bold section".length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); - String html = SpannedToHtmlConverter.convert(spanned, displayDensity); + SpannedToHtmlConverter.HtmlAndCss htmlAndCss = + SpannedToHtmlConverter.convert(spanned, displayDensity); - assertThat(html).isEqualTo("String with italic and bold section"); + assertThat(htmlAndCss.cssRuleSets).isEmpty(); + assertThat(htmlAndCss.html).isEqualTo("String with italic and bold section"); } }