Store the language internally instead of using lang attribute for WebVTT nodes
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Feb 2013 23:33:15 +0000 (23:33 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Feb 2013 23:33:15 +0000 (23:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=108858

Patch by Dima Gorbik <dgorbik@apple.com> on 2013-02-06
Reviewed by Eric Carlson.

Source/WebCore:

Only language webvtt elements should have a lang attribute so we have to store
the language internally in the element. Refactored the code to make
computeInheritedLanguage virtual.

Existing tests were modified to cover this case.

* css/SelectorChecker.cpp:
(WebCore::SelectorChecker::checkOne):
* html/track/WebVTTElement.cpp:
(WebCore::WebVTTElement::WebVTTElement):
(WebCore::WebVTTElement::cloneElementWithoutAttributesAndChildren):
(WebCore::WebVTTElement::createEquivalentHTMLElement): clone the internal language property.
* html/track/WebVTTElement.h:
(WebCore::WebVTTElement::language):
(WebCore::WebVTTElement::setLanguage):
* html/track/WebVTTParser.cpp: only set the lang attribute for language objects.
(WebCore::WebVTTParser::constructTreeFromToken):

LayoutTests:

* media/track/captions-webvtt/styling-lang.vtt:
* media/track/track-css-matching-lang-expected.txt:
* media/track/track-css-matching-lang.html:

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

LayoutTests/ChangeLog
LayoutTests/media/track/captions-webvtt/styling-lang.vtt
LayoutTests/media/track/track-css-matching-lang-expected.txt
LayoutTests/media/track/track-css-matching-lang.html
Source/WebCore/ChangeLog
Source/WebCore/css/SelectorChecker.cpp
Source/WebCore/html/track/WebVTTElement.cpp
Source/WebCore/html/track/WebVTTElement.h
Source/WebCore/html/track/WebVTTParser.cpp

index 206c395..1a89207 100644 (file)
@@ -1,3 +1,14 @@
+2013-02-06  Dima Gorbik  <dgorbik@apple.com>
+
+        Store the language internally instead of using lang attribute for WebVTT nodes
+        https://bugs.webkit.org/show_bug.cgi?id=108858
+
+        Reviewed by Eric Carlson.
+
+        * media/track/captions-webvtt/styling-lang.vtt:
+        * media/track/track-css-matching-lang-expected.txt:
+        * media/track/track-css-matching-lang.html:
+
 2013-02-06  Levi Weintraub  <leviw@chromium.org>
 
         Negative text indents can break RenderBlock's inline maximum preferred width calculation
index e7d091a..64b2268 100644 (file)
@@ -2,8 +2,4 @@ WEBVTT
 
 1
 00:00.000 --> 00:00.500
-<lang en><c>English<lang ru><c> Русский<lang en><c> English again</c></lang></c></lang></c></lang>
-
-2
-00:00.500 --> 00:01.000
-<lang en><i>English<lang ru><b> Русский<lang en><u> English again</u></lang></b></lang></i></lang>
+<lang en><b>English<lang ru><c> Русский<lang en><i> English again</i></lang></c></lang></b></lang>
index 03578fc..3ee4cc5 100644 (file)
@@ -1,12 +1,6 @@
-Test that cues are being matched properly by the lang attribute.
+Test that cues are being matched properly by the lang attribute and :lang() pseudo class.
 EVENT(canplaythrough)
 EVENT(seeked)
-EXPECTED (getComputedStyle(cueNode).color == 'rgb(0, 128, 0)') OK
-EXPECTED (getComputedStyle(cueNode).color == 'rgb(255, 0, 0)') OK
-EXPECTED (getComputedStyle(cueNode).color == 'rgb(0, 128, 0)') OK
-
-RUN(video.currentTime = 0.6)
-EVENT(seeked)
 EXPECTED (getComputedStyle(cueNode).color == 'rgb(128, 0, 128)') OK
 EXPECTED (getComputedStyle(cueNode).color == 'rgb(0, 255, 0)') OK
 EXPECTED (getComputedStyle(cueNode).color == 'rgb(128, 0, 128)') OK
index fbee929..3435c17 100644 (file)
@@ -9,19 +9,17 @@
 
         <style>
         ::cue(:lang(ru)) { color: lime; }
-        ::cue(:lang(en)) { color: purple; }
-        ::cue(c[lang="ru"]) { color: red; }
-        ::cue(c[lang="en"]) { color: green; }
+        ::cue(lang[lang="en"]) { color: purple; }
+        ::cue(c[lang="ru"]) { color: red; } /* Shouldn't work, no attribute 'lang' for 'c'. */
         </style>
 
         <script>
 
         var cueNode;
         var seekedCount = 0;
-        var seekTimes = [0.1, 0.6];
+        var seekTimes = [0.1];
 
-        var info = [["rgb(0, 128, 0)", "rgb(255, 0, 0)", "rgb(0, 128, 0)"],
-                    ["rgb(128, 0, 128)", "rgb(0, 255, 0)", "rgb(128, 0, 128)"]];
+        var info = [["rgb(128, 0, 128)", "rgb(0, 255, 0)", "rgb(128, 0, 128)"]];
 
         function seeked()
         {
@@ -45,7 +43,7 @@
 
         function loaded()
         {
-            consoleWrite("Test that cues are being matched properly by the lang attribute.");
+            consoleWrite("Test that cues are being matched properly by the lang attribute and :lang() pseudo class.");
             findMediaElement();
             video.src = findMediaFile('video', '../content/test');
             video.id = "testvideo";
index e9d7827..78cc0ec 100644 (file)
@@ -1,3 +1,28 @@
+2013-02-06  Dima Gorbik  <dgorbik@apple.com>
+
+        Store the language internally instead of using lang attribute for WebVTT nodes
+        https://bugs.webkit.org/show_bug.cgi?id=108858
+
+        Reviewed by Eric Carlson.
+
+        Only language webvtt elements should have a lang attribute so we have to store
+        the language internally in the element. Refactored the code to make 
+        computeInheritedLanguage virtual.
+
+        Existing tests were modified to cover this case.
+
+        * css/SelectorChecker.cpp:
+        (WebCore::SelectorChecker::checkOne):
+        * html/track/WebVTTElement.cpp:
+        (WebCore::WebVTTElement::WebVTTElement):
+        (WebCore::WebVTTElement::cloneElementWithoutAttributesAndChildren):
+        (WebCore::WebVTTElement::createEquivalentHTMLElement): clone the internal language property.
+        * html/track/WebVTTElement.h:
+        (WebCore::WebVTTElement::language):
+        (WebCore::WebVTTElement::setLanguage):
+        * html/track/WebVTTParser.cpp: only set the lang attribute for language objects.
+        (WebCore::WebVTTParser::constructTreeFromToken):
+
 2013-02-06  Levi Weintraub  <leviw@chromium.org>
 
         Negative text indents can break RenderBlock's inline maximum preferred width calculation
index 3ca64dd..30fc49a 100644 (file)
@@ -822,7 +822,7 @@ bool SelectorChecker::checkOne(const SelectorCheckingContext& context, const Sib
                 AtomicString value;
 #if ENABLE(VIDEO_TRACK)
                 if (element->isWebVTTElement())
-                    value = element->getAttribute(langAttr);
+                    value = toWebVTTElement(element)->language();
                 else
 #endif
                     value = element->computeInheritedLanguage();
index d677272..e063e15 100644 (file)
@@ -81,7 +81,9 @@ PassRefPtr<WebVTTElement> WebVTTElement::create(WebVTTNodeType nodeType, Documen
 
 PassRefPtr<Element> WebVTTElement::cloneElementWithoutAttributesAndChildren()
 {
-    return create(static_cast<WebVTTNodeType>(m_webVTTNodeType), document());
+    RefPtr<WebVTTElement> clone = create(static_cast<WebVTTNodeType>(m_webVTTNodeType), document());
+    clone->setLanguage(m_language);
+    return clone;
 }
 
 PassRefPtr<HTMLElement> WebVTTElement::createEquivalentHTMLElement(Document* document)
@@ -93,6 +95,7 @@ PassRefPtr<HTMLElement> WebVTTElement::createEquivalentHTMLElement(Document* doc
     case WebVTTNodeTypeVoice:
         htmlElement = HTMLElement::create(HTMLNames::spanTag, document);
         htmlElement.get()->setAttribute(HTMLNames::titleAttr, getAttribute(voiceAttributeName()));
+        htmlElement.get()->setAttribute(HTMLNames::langAttr, getAttribute(langAttributeName()));
         break;
     case WebVTTNodeTypeItalic:
         htmlElement = HTMLElement::create(HTMLNames::iTag, document);
@@ -113,7 +116,6 @@ PassRefPtr<HTMLElement> WebVTTElement::createEquivalentHTMLElement(Document* doc
         ASSERT_NOT_REACHED();
     }
 
-    htmlElement.get()->setAttribute(HTMLNames::langAttr, getAttribute(langAttributeName()));
     htmlElement.get()->setAttribute(HTMLNames::classAttr, getAttribute(HTMLNames::classAttr));
     return htmlElement;
 }
index d400ad6..94a3f6b 100644 (file)
@@ -56,6 +56,8 @@ public:
     void setIsPastNode(bool value) { m_isPastNode = value; }
 
     virtual bool isWebVTTElement() const OVERRIDE { return true; }
+    AtomicString language() const { return m_language; }
+    void setLanguage(AtomicString value) { m_language = value; }
 
     static const QualifiedName& voiceAttributeName()
     {
@@ -76,6 +78,7 @@ private:
     unsigned m_isPastNode : 1;
     unsigned m_webVTTNodeType : 4;
     
+    AtomicString m_language;
 };
 
 inline WebVTTElement* toWebVTTElement(Node* node)
index f835060..1e626e5 100644 (file)
@@ -396,12 +396,12 @@ void WebVTTParser::constructTreeFromToken(Document* document)
 
             if (child->webVTTNodeType() == WebVTTNodeTypeVoice)
                 child->setAttribute(WebVTTElement::voiceAttributeName(), AtomicString(m_token.annotation().data(), m_token.annotation().size()));
-            else if (child->webVTTNodeType() == WebVTTNodeTypeLanguage)
+            else if (child->webVTTNodeType() == WebVTTNodeTypeLanguage) {
                 m_languageStack.append(AtomicString(m_token.annotation().data(), m_token.annotation().size()));
-
-            if (!m_languageStack.isEmpty())
                 child->setAttribute(WebVTTElement::langAttributeName(), m_languageStack.last());
-
+            }
+            if (!m_languageStack.isEmpty())
+                child->setLanguage(m_languageStack.last());
             m_currentNode->parserAppendChild(child);
             m_currentNode = child;
         }