Use a separate backdrop element to allow cues to have highlight and background color

https://bugs.webkit.org/show_bug.cgi?id=134821
<rdar://problem/15999721>

Reviewed by Eric Carlson.

Source/WebCore:
Add a new <div> element wrapping the existing cue <span>. This allows
us to have a highlight on the cue (in the <span> background), as well
as an overall background color.

* Modules/mediacontrols/mediaControlsApple.css:
(video::-webkit-media-text-track-display-backdrop): New markup for
the backdrop element of the caption.
* html/track/VTTCue.cpp:
(WebCore::VTTCue::cueBackdropShadowPseudoId): Added to
allow user customization of the cue backdrop.
(WebCore::VTTCue::initialize): Rename the old "m_cueBackgroundBox" to
"m_cueHighlightBox" and add a new "m_cueBackdropBox" member.
(WebCore::VTTCue::updateDisplayTree): Update for m_cueHighlightBox.
(WebCore::VTTCue::getDisplayTree): Make m_cueHighlightBox a child
of the new m_cueBackdropBox element, and add m_cueBackdropBox to
the display tree.
* html/track/VTTCue.h:
(WebCore::VTTCue::element):
* page/CaptionUserPreferencesMediaAF.cpp:
(WebCore::CaptionUserPreferencesMediaAF::setInterestedInCaptionPreferenceChanges):
Fix for missing caption style updates. Even if we are already
listening for caption changes, we still want to update the new
instance's style sheet to match.
(WebCore::CaptionUserPreferencesMediaAF::captionsStyleSheetOverride):
* rendering/RenderVTTCue.cpp:
(WebCore::RenderVTTCue::initializeLayoutParameters): Take the new
<div> into consideration when looking for the Cue text element.

LayoutTests:
Updated tests for new formatting logic.

* platform/mac/media/track/track-cue-rendering-horizontal-expected.png:
* platform/mac/media/track/track-cue-rendering-horizontal-expected.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@171004 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 111df99..c0710c4 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2014-07-11  Brent Fulgham  <bfulgham@apple.com>
+
+        Use a separate backdrop element to allow cues to have highlight and background color
+        https://bugs.webkit.org/show_bug.cgi?id=134821
+        <rdar://problem/15999721>
+
+        Reviewed by Eric Carlson.
+
+        Updated tests for new formatting logic.
+
+        * platform/mac/media/track/track-cue-rendering-horizontal-expected.png:
+        * platform/mac/media/track/track-cue-rendering-horizontal-expected.txt:
+
 2014-07-11  Zalan Bujtas  <zalan@apple.com>
 
         Subpixel layout: return integral results for offset*, client*, scroll* by default.

diff --git a/LayoutTests/platform/mac/media/track/track-cue-rendering-horizontal-expected.png b/LayoutTests/platform/mac/media/track/track-cue-rendering-horizontal-expected.png
index 5b4ae3f..c615e17 100644 (file)
Binary files a/LayoutTests/platform/mac/media/track/track-cue-rendering-horizontal-expected.png and b/LayoutTests/platform/mac/media/track/track-cue-rendering-horizontal-expected.png differ

diff --git a/LayoutTests/platform/mac/media/track/track-cue-rendering-horizontal-expected.txt b/LayoutTests/platform/mac/media/track/track-cue-rendering-horizontal-expected.txt
index 49bb87f..b2e25fd 100644 (file)
--- a/LayoutTests/platform/mac/media/track/track-cue-rendering-horizontal-expected.txt
+++ b/LayoutTests/platform/mac/media/track/track-cue-rendering-horizontal-expected.txt
@@ -26,31 +26,37 @@ layer at (8,8) size 320x240
   RenderBlock (relative positioned) {DIV} at (0,0) size 320x240 [color=#FFFFFF]
 layer at (8,8) size 320x14
   RenderBlock (positioned) {DIV} at (0,0) size 320x14
-    RenderInline {SPAN} at (0,0) size 276x14 [bgcolor=#000000CC]
-      RenderText {#text} at (22,0) size 276x14
-        text run at (22,0) width 276: "Cue 1: should be positioned at the top of the video."
+    RenderBlock {DIV} at (22,0) size 276x14
+      RenderInline {SPAN} at (0,0) size 275x14 [bgcolor=#000000CC]
+        RenderText {#text} at (0,0) size 275x14
+          text run at (0,0) width 275: "Cue 1: should be positioned at the top of the video."
 layer at (8,22) size 320x14
   RenderBlock (positioned) {DIV} at (0,14) size 320x14
-    RenderInline {SPAN} at (0,0) size 304x14 [bgcolor=#000000CC]
-      RenderText {#text} at (8,0) size 304x14
-        text run at (8,0) width 304: "Cue 2: should be the second cue and not overlap cue 1."
+    RenderBlock {DIV} at (8,0) size 304x14
+      RenderInline {SPAN} at (0,0) size 303x14 [bgcolor=#000000CC]
+        RenderText {#text} at (0,0) size 303x14
+          text run at (0,0) width 303: "Cue 2: should be the second cue and not overlap cue 1."
 layer at (8,36) size 320x14
   RenderBlock (positioned) {DIV} at (0,28) size 320x14
-    RenderInline {SPAN} at (0,0) size 296x14 [bgcolor=#000000CC]
-      RenderText {#text} at (12,0) size 296x14
-        text run at (12,0) width 296: "Cue 3: should become the third line from top to bottom."
+    RenderBlock {DIV} at (12,0) size 296x14
+      RenderInline {SPAN} at (0,0) size 296x14 [bgcolor=#000000CC]
+        RenderText {#text} at (0,0) size 296x14
+          text run at (0,0) width 296: "Cue 3: should become the third line from top to bottom."
 layer at (8,78) size 320x14
   RenderBlock (positioned) {DIV} at (0,70) size 320x14
-    RenderInline {SPAN} at (0,0) size 288x14 [bgcolor=#000000CC]
-      RenderText {#text} at (16,0) size 288x14
-        text run at (16,0) width 288: "Cue 4: should be fixed positioned around the middle."
+    RenderBlock {DIV} at (16,0) size 288x14
+      RenderInline {SPAN} at (0,0) size 287x14 [bgcolor=#000000CC]
+        RenderText {#text} at (0,0) size 287x14
+          text run at (0,0) width 287: "Cue 4: should be fixed positioned around the middle."
 layer at (8,234) size 320x14
   RenderBlock (positioned) {DIV} at (0,226) size 320x14
-    RenderInline {SPAN} at (0,0) size 292x14 [bgcolor=#000000CC]
-      RenderText {#text} at (14,0) size 292x14
-        text run at (14,0) width 292: "Cue 5: should be displayed at the bottom of the video."
+    RenderBlock {DIV} at (14,0) size 292x14
+      RenderInline {SPAN} at (0,0) size 291x14 [bgcolor=#000000CC]
+        RenderText {#text} at (0,0) size 291x14
+          text run at (0,0) width 291: "Cue 5: should be displayed at the bottom of the video."
 layer at (8,220) size 320x14
   RenderBlock (positioned) {DIV} at (0,212) size 320x14
-    RenderInline {SPAN} at (0,0) size 280x14 [bgcolor=#000000CC]
-      RenderText {#text} at (20,0) size 280x14
-        text run at (20,0) width 280: "Cue 6: should be on top of bottom positioned cue 5."
+    RenderBlock {DIV} at (20,0) size 280x14
+      RenderInline {SPAN} at (0,0) size 279x14 [bgcolor=#000000CC]
+        RenderText {#text} at (0,0) size 279x14
+          text run at (0,0) width 279: "Cue 6: should be on top of bottom positioned cue 5."

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index bc67327..3a0f941 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,39 @@
+2014-07-10  Brent Fulgham  <bfulgham@apple.com>
+
+        Use a separate backdrop element to allow cues to have highlight and background color
+        https://bugs.webkit.org/show_bug.cgi?id=134821
+        <rdar://problem/15999721>
+
+        Reviewed by Eric Carlson.
+
+        Add a new <div> element wrapping the existing cue <span>. This allows
+        us to have a highlight on the cue (in the <span> background), as well
+        as an overall background color.
+
+        * Modules/mediacontrols/mediaControlsApple.css:
+        (video::-webkit-media-text-track-display-backdrop): New markup for
+        the backdrop element of the caption.
+        * html/track/VTTCue.cpp:
+        (WebCore::VTTCue::cueBackdropShadowPseudoId): Added to
+        allow user customization of the cue backdrop.
+        (WebCore::VTTCue::initialize): Rename the old "m_cueBackgroundBox" to
+        "m_cueHighlightBox" and add a new "m_cueBackdropBox" member.
+        (WebCore::VTTCue::updateDisplayTree): Update for m_cueHighlightBox.
+        (WebCore::VTTCue::getDisplayTree): Make m_cueHighlightBox a child
+        of the new m_cueBackdropBox element, and add m_cueBackdropBox to
+        the display tree.
+        * html/track/VTTCue.h:
+        (WebCore::VTTCue::element):
+        * page/CaptionUserPreferencesMediaAF.cpp:
+        (WebCore::CaptionUserPreferencesMediaAF::setInterestedInCaptionPreferenceChanges):
+        Fix for missing caption style updates. Even if we are already
+        listening for caption changes, we still want to update the new
+        instance's style sheet to match.
+        (WebCore::CaptionUserPreferencesMediaAF::captionsStyleSheetOverride):
+        * rendering/RenderVTTCue.cpp:
+        (WebCore::RenderVTTCue::initializeLayoutParameters): Take the new
+        <div> into consideration when looking for the Cue text element.
+
 2014-07-11  Zalan Bujtas  <zalan@apple.com>
 
         Subpixel layout: return integral results for offset*, client*, scroll* by default.

diff --git a/Source/WebCore/Modules/mediacontrols/mediaControlsApple.css b/Source/WebCore/Modules/mediacontrols/mediaControlsApple.css
index 5c68a5a..ce95a15 100644 (file)
--- a/Source/WebCore/Modules/mediacontrols/mediaControlsApple.css
+++ b/Source/WebCore/Modules/mediacontrols/mediaControlsApple.css
@@ -728,6 +728,10 @@ video::-webkit-media-text-track-display {
     font: 22px sans-serif;
 }
 
+video::-webkit-media-text-track-display-backdrop {
+    display: inline-block;
+}
+
 video::cue(:future) {
     color: gray;
 }

diff --git a/Source/WebCore/Modules/mediacontrols/mediaControlsiOS.css b/Source/WebCore/Modules/mediacontrols/mediaControlsiOS.css
index bd610a9..05b4cbd 100644 (file)
--- a/Source/WebCore/Modules/mediacontrols/mediaControlsiOS.css
+++ b/Source/WebCore/Modules/mediacontrols/mediaControlsiOS.css
@@ -380,6 +380,10 @@ video::-webkit-media-text-track-display {
     font: 22px sans-serif;
 }
 
+video::-webkit-media-text-track-display-backdrop {
+    display: inline-block;
+}
+
 video::cue(:future) {
     color: gray;
 }

diff --git a/Source/WebCore/html/track/VTTCue.cpp b/Source/WebCore/html/track/VTTCue.cpp
index 869ed83..35b4df0 100644 (file)
--- a/Source/WebCore/html/track/VTTCue.cpp
+++ b/Source/WebCore/html/track/VTTCue.cpp
@@ -216,6 +216,12 @@ RenderPtr<RenderElement> VTTCueBox::createElementRenderer(PassRef<RenderStyle> s
 
 // ----------------------------
 
+const AtomicString& VTTCue::cueBackdropShadowPseudoId()
+{
+    DEPRECATED_DEFINE_STATIC_LOCAL(const AtomicString, cueBackdropShadowPseudoId, ("-webkit-media-text-track-display-backdrop", AtomicString::ConstructFromLiteral));
+    return cueBackdropShadowPseudoId;
+}
+
 PassRefPtr<VTTCue> VTTCue::create(ScriptExecutionContext& context, double start, double end, const String& content)
 {
     return adoptRef(new VTTCue(context, start, end, content));
@@ -262,7 +268,8 @@ void VTTCue::initialize(ScriptExecutionContext& context)
     m_writingDirection = Horizontal;
     m_cueAlignment = Middle;
     m_webVTTNodeTree = nullptr;
-    m_cueBackgroundBox = HTMLSpanElement::create(spanTag, toDocument(context));
+    m_cueBackdropBox = HTMLDivElement::create(toDocument(context));
+    m_cueHighlightBox = HTMLSpanElement::create(spanTag, toDocument(context));
     m_displayDirection = CSSValueLtr;
     m_displaySize = 0;
     m_snapToLines = true;
@@ -757,7 +764,7 @@ void VTTCue::updateDisplayTree(double movieTime)
         return;
 
     // Clear the contents of the set.
-    m_cueBackgroundBox->removeChildren();
+    m_cueHighlightBox->removeChildren();
 
     // Update the two sets containing past and future WebVTT objects.
     RefPtr<DocumentFragment> referenceTree = createCueRenderingTree();
@@ -765,7 +772,7 @@ void VTTCue::updateDisplayTree(double movieTime)
         return;
 
     markFutureAndPastNodes(referenceTree.get(), startTime(), movieTime);
-    m_cueBackgroundBox->appendChild(referenceTree);
+    m_cueHighlightBox->appendChild(referenceTree);
 }
 
 VTTCueBox* VTTCue::getDisplayTree(const IntSize& videoSize)
@@ -788,9 +795,12 @@ VTTCueBox* VTTCue::getDisplayTree(const IntSize& videoSize)
     // 'display' property has the value 'inline'. This is the WebVTT cue
     // background box.
 
-    // Note: This is contained by default in m_cueBackgroundBox.
-    m_cueBackgroundBox->setPseudo(cueShadowPseudoId());
-    displayTree->appendChild(m_cueBackgroundBox, ASSERT_NO_EXCEPTION);
+    // Note: This is contained by default in m_cueHighlightBox.
+    m_cueHighlightBox->setPseudo(cueShadowPseudoId());
+
+    m_cueBackdropBox->setPseudo(cueBackdropShadowPseudoId());
+    m_cueBackdropBox->appendChild(m_cueHighlightBox, ASSERT_NO_EXCEPTION);
+    displayTree->appendChild(m_cueBackdropBox, ASSERT_NO_EXCEPTION);
 
     // FIXME(BUG 79916): Runs of children of WebVTT Ruby Objects that are not
     // WebVTT Ruby Text Objects must be wrapped in anonymous boxes whose

diff --git a/Source/WebCore/html/track/VTTCue.h b/Source/WebCore/html/track/VTTCue.h
index 877247d..b3de13a 100644 (file)
--- a/Source/WebCore/html/track/VTTCue.h
+++ b/Source/WebCore/html/track/VTTCue.h
@@ -42,6 +42,7 @@
 namespace WebCore {
 
 class DocumentFragment;
+class HTMLDivElement;
 class HTMLSpanElement;
 class ScriptExecutionContext;
 class VTTCue;
@@ -74,6 +75,8 @@ public:
     static PassRefPtr<VTTCue> create(ScriptExecutionContext&, double start, double end, const String&);
     static PassRefPtr<VTTCue> create(ScriptExecutionContext&, const WebVTTCueData&);
 
+    static const AtomicString& cueBackdropShadowPseudoId();
+
     virtual ~VTTCue();
 
     const String& vertical() const;
@@ -113,7 +116,7 @@ public:
 
     bool hasDisplayTree() const { return m_displayTree; }
     VTTCueBox* getDisplayTree(const IntSize& videoSize);
-    HTMLSpanElement* element() const { return m_cueBackgroundBox.get(); }
+    HTMLSpanElement* element() const { return m_cueHighlightBox.get(); }
 
     void updateDisplayTree(double);
     void removeDisplayTree();
@@ -202,7 +205,8 @@ private:
 #endif
 
     RefPtr<DocumentFragment> m_webVTTNodeTree;
-    RefPtr<HTMLSpanElement> m_cueBackgroundBox;
+    RefPtr<HTMLSpanElement> m_cueHighlightBox;
+    RefPtr<HTMLDivElement> m_cueBackdropBox;
     RefPtr<VTTCueBox> m_displayTree;
 
     CSSValueID m_displayDirection;

diff --git a/Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp b/Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp
index efe2e58..7ef57b4 100644 (file)
--- a/Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp
+++ b/Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp
@@ -234,8 +234,9 @@ void CaptionUserPreferencesMediaAF::setInterestedInCaptionPreferenceChanges()
     if (!m_listeningForPreferenceChanges) {
         m_listeningForPreferenceChanges = true;
         CFNotificationCenterAddObserver(CFNotificationCenterGetLocalCenter(), this, userCaptionPreferencesChangedNotificationCallback, kMAXCaptionAppearanceSettingsChangedNotification, 0, CFNotificationSuspensionBehaviorCoalesce);
-        updateCaptionStyleSheetOveride();
     }
+
+    updateCaptionStyleSheetOveride();
 }
 
 void CaptionUserPreferencesMediaAF::captionPreferencesChanged()
@@ -540,13 +541,14 @@ String CaptionUserPreferencesMediaAF::captionsStyleSheetOverride() const
     String windowCornerRadius = windowRoundedCornerRadiusCSS();
     if (!windowColor.isEmpty() || !windowCornerRadius.isEmpty()) {
         captionsOverrideStyleSheet.append(" video::");
-        captionsOverrideStyleSheet.append(VTTCueBox::vttCueBoxShadowPseudoId());
+        captionsOverrideStyleSheet.append(VTTCue::cueBackdropShadowPseudoId());
         captionsOverrideStyleSheet.append('{');
         
         if (!windowColor.isEmpty())
             captionsOverrideStyleSheet.append(windowColor);
-        if (!windowCornerRadius.isEmpty())
+        if (!windowCornerRadius.isEmpty()) {
             captionsOverrideStyleSheet.append(windowCornerRadius);
+        }
         
         captionsOverrideStyleSheet.append('}');
     }

diff --git a/Source/WebCore/rendering/RenderVTTCue.cpp b/Source/WebCore/rendering/RenderVTTCue.cpp
index 2e26cca..4ccfb34 100644 (file)
--- a/Source/WebCore/rendering/RenderVTTCue.cpp
+++ b/Source/WebCore/rendering/RenderVTTCue.cpp
@@ -74,7 +74,13 @@ bool RenderVTTCue::initializeLayoutParameters(InlineFlowBox*& firstLineBox, Layo
     ASSERT(firstChild());
 
     RenderBlock* parentBlock = containingBlock();
-    firstLineBox = toRenderInline(firstChild())->firstLineBox();
+
+    // firstChild() returns the wrapping (backdrop) <div>. The cue object is
+    // the <div>'s first child.
+    RenderObject* firstChild = this->firstChild();
+    RenderElement* backdropElement = toRenderElement(firstChild);
+    
+    firstLineBox = toRenderInline(backdropElement->firstChild())->firstLineBox();
     if (!firstLineBox)
         firstLineBox = this->firstRootBox();
 
@@ -334,7 +340,13 @@ void RenderVTTCue::repositionCueSnapToLinesSet()
 void RenderVTTCue::repositionGenericCue()
 {
     ASSERT(firstChild());
-    InlineFlowBox* firstLineBox = toRenderInline(firstChild())->firstLineBox();
+
+    // firstChild() returns the wrapping (backdrop) <div>. The cue object is
+    // the <div>'s first child.
+    RenderObject* firstChild = this->firstChild();
+    RenderElement* backdropElement = toRenderElement(firstChild);
+    
+    InlineFlowBox* firstLineBox = toRenderInline(backdropElement->firstChild())->firstLineBox();
     if (static_cast<TextTrackCueGeneric*>(m_cue)->useDefaultPosition() && firstLineBox) {
         LayoutUnit parentWidth = containingBlock()->logicalWidth();
         LayoutUnit width = firstLineBox->width();

diff --git a/WebKit.xcworkspace/xcshareddata/xcschemes/All Source.xcscheme b/WebKit.xcworkspace/xcshareddata/xcschemes/All Source.xcscheme
index f12064f..4986e8e 100644 (file)
--- a/WebKit.xcworkspace/xcshareddata/xcschemes/All Source.xcscheme
+++ b/WebKit.xcworkspace/xcshareddata/xcschemes/All Source.xcscheme
@@ -130,6 +130,15 @@
       buildConfiguration = "Debug">
       <Testables>
       </Testables>
+      <MacroExpansion>
+         <BuildableReference
+            BuildableIdentifier = "primary"
+            BlueprintIdentifier = "8D1107260486CEB800E47090"
+            BuildableName = "MiniBrowser.app"
+            BlueprintName = "MiniBrowser"
+            ReferencedContainer = "container:Tools/MiniBrowser/MiniBrowser.xcodeproj">
+         </BuildableReference>
+      </MacroExpansion>
    </TestAction>
    <LaunchAction
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
@@ -140,9 +149,15 @@
       ignoresPersistentStateOnLaunch = "YES"
       debugDocumentVersioning = "YES"
       allowLocationSimulation = "YES">
-      <PathRunnable
-         FilePath = "/Applications/Safari.app/Contents/MacOS/SafariForWebKitDevelopment">
-      </PathRunnable>
+      <BuildableProductRunnable>
+         <BuildableReference
+            BuildableIdentifier = "primary"
+            BlueprintIdentifier = "8D1107260486CEB800E47090"
+            BuildableName = "MiniBrowser.app"
+            BlueprintName = "MiniBrowser"
+            ReferencedContainer = "container:Tools/MiniBrowser/MiniBrowser.xcodeproj">
+         </BuildableReference>
+      </BuildableProductRunnable>
       <AdditionalOptions>
       </AdditionalOptions>
    </LaunchAction>