Title: [102313] trunk
Revision
102313
Author
ba...@chromium.org
Date
2011-12-07 21:38:26 -0800 (Wed, 07 Dec 2011)

Log Message

Refactor CSSParser::parseFontFaceSrc()
https://bugs.webkit.org/show_bug.cgi?id=73989

Reviewed by Darin Adler.

Source/WebCore:

Test: fast/css/font-face-src-parsing.html

* css/CSSParser.cpp:
(WebCore::CSSParser::parseFontFaceSrcURI): Added.
(WebCore::CSSParser::parseFontFaceSrcLocal): Added.
(WebCore::CSSParser::parseFontFaceSrc): Rewrote.
* css/CSSParser.h:

LayoutTests:

Added a test that checks parsing result of the src descriptor of @font-face.

* fast/css/font-face-src-parsing-expected.txt: Added.
* fast/css/font-face-src-parsing.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (102312 => 102313)


--- trunk/LayoutTests/ChangeLog	2011-12-08 05:23:42 UTC (rev 102312)
+++ trunk/LayoutTests/ChangeLog	2011-12-08 05:38:26 UTC (rev 102313)
@@ -1,5 +1,17 @@
 2011-12-07  Kenichi Ishibashi  <ba...@chromium.org>
 
+        Refactor CSSParser::parseFontFaceSrc()
+        https://bugs.webkit.org/show_bug.cgi?id=73989
+
+        Reviewed by Darin Adler.
+
+        Added a test that checks parsing result of the src descriptor of @font-face.
+
+        * fast/css/font-face-src-parsing-expected.txt: Added.
+        * fast/css/font-face-src-parsing.html: Added.
+
+2011-12-07  Kenichi Ishibashi  <ba...@chromium.org>
+
         Unreviewed chromium expectations update.
 
         * platform/chromium/test_expectations.txt:

Added: trunk/LayoutTests/fast/css/font-face-src-parsing-expected.txt (0 => 102313)


--- trunk/LayoutTests/fast/css/font-face-src-parsing-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/css/font-face-src-parsing-expected.txt	2011-12-08 05:38:26 UTC (rev 102313)
@@ -0,0 +1,51 @@
+Test parsing the src descriptor of @font-face.
+
+Valid rules form the stylesheet:
+
+@font-face { src: url(font.ttf); }
+@font-face { src: url(http://foo/bar/font.ttf); }
+@font-face { src: url(font.svg#id); }
+@font-face { src: url(font.ttf) format(truetype); }
+@font-face { src: url(font.woff) format(woff), local(font2); }
+@font-face { src: url(font2.ttf); }
+@font-face { src: url(font2.ttf) format(truetype); }
+@font-face { src: url(font3.otf) format(opentype), local(foo bar); }
+@font-face { src: local(foo); }
+@font-face { src: local(font), local(foo bar); }
+@font-face { src: local(foo); }
+@font-face { src: local(foo bar); }
+Expected result for valid rules:
+
+@font-face { src: url(font.ttf); }
+@font-face { src: url(http://foo/bar/font.ttf); }
+@font-face { src: url(font.svg#id); }
+@font-face { src: url(font.ttf) format(truetype); }
+@font-face { src: url(font.woff) format(woff), local(font2); }
+@font-face { src: url(font2.ttf); }
+@font-face { src: url(font2.ttf) format(truetype); }
+@font-face { src: url(font3.otf) format(opentype), local(foo bar); }
+@font-face { src: local(foo); }
+@font-face { src: local(font), local(foo bar); }
+@font-face { src: local(foo); }
+@font-face { src: local(foo bar); }
+Invalid rules form the stylesheet:
+
+@font-face { }
+@font-face { }
+@font-face { }
+@font-face { }
+@font-face { }
+@font-face { }
+@font-face { }
+@font-face { }
+Expected result for invalid rules:
+
+@font-face { }
+@font-face { }
+@font-face { }
+@font-face { }
+@font-face { }
+@font-face { }
+@font-face { }
+@font-face { }
+

Added: trunk/LayoutTests/fast/css/font-face-src-parsing.html (0 => 102313)


--- trunk/LayoutTests/fast/css/font-face-src-parsing.html	                        (rev 0)
+++ trunk/LayoutTests/fast/css/font-face-src-parsing.html	2011-12-08 05:38:26 UTC (rev 102313)
@@ -0,0 +1,122 @@
+<!-- Valid src descriptor rules. -->
+<style>
+@font-face {
+    src: url(font.ttf);
+}
+@font-face {
+    src: url(http://foo/bar/font.ttf);
+}
+@font-face {
+    src: url(font.svg#id);
+}
+@font-face {
+    src: url(font.ttf) format("truetype");
+}
+@font-face {
+    src: url(font.woff) format("woff"), local(font2);
+}
+@font-face {
+    src: url(font.ttf) format("truetype"), url(font2.ttf);
+}
+@font-face {
+    src: url(font.ttf), url(font2.ttf) format("truetype");
+}
+@font-face {
+    src: url(font.ttf), local(font2), url(font3.otf) format("opentype"), local(foo bar);
+}
+@font-face {
+    src: local(foo);
+}
+@font-face {
+    src: local(font), local(foo bar);
+}
+@font-face {
+    src: local("foo");
+}
+@font-face {
+    src: local("foo bar");
+}
+</style>
+
+<!-- Invalid src descriptor rules. -->
+<style>
+@font-face {
+    src: url(font.ttf invalid);
+}
+@font-face {
+    src: url(font.ttf),;
+}
+@font-face {
+    src: url(font.ttf), local(font2),;
+}
+@font-face {
+    src: ,local(font);
+}
+@font-face {
+    src: url(font.ttf), invalid();
+}
+@font-face {
+    src: url(foo.ttf) invalid;
+}
+
+@font-face {
+    src: url(foo.ttf), invalid;
+}
+@font-face {
+    src: url(foo.ttf) "invalid";
+}
+</style>
+<p>Test parsing the src descriptor of @font-face.</p>
+
+<p>Valid rules form the stylesheet:</p>
+<div id="validResult"></div>
+
+<p>Expected result for valid rules:</p>
+<pre id="validExpected">@font-face { src: url(font.ttf); }
+@font-face { src: url(http://foo/bar/font.ttf); }
+@font-face { src: url(font.svg#id); }
+@font-face { src: url(font.ttf) format(truetype); }
+@font-face { src: url(font.woff) format(woff), local(font2); }
+@font-face { src: url(font2.ttf); }
+@font-face { src: url(font2.ttf) format(truetype); }
+@font-face { src: url(font3.otf) format(opentype), local(foo bar); }
+@font-face { src: local(foo); }
+@font-face { src: local(font), local(foo bar); }
+@font-face { src: local(foo); }
+@font-face { src: local(foo bar); }
+</pre>
+
+<p>Invalid rules form the stylesheet:</p>
+<pre id="invalidResult"></pre>
+
+<p>Expected result for invalid rules:</p>
+<pre id="invalidExpected">@font-face { }
+@font-face { }
+@font-face { }
+@font-face { }
+@font-face { }
+@font-face { }
+@font-face { }
+@font-face { }
+</pre>
+
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+var validRules = document.styleSheets[0].cssRules;
+var result = '';
+for (var i = 0; i < validRules.length; ++i) {
+    var itemText = validRules.item(i).cssText.replace(/file:.*LayoutTests\/fast\/css\//, '');
+    result += itemText + '\n';
+}
+document.getElementById('validResult').innerText = result;
+
+var invalidRules = document.styleSheets[1].cssRules;
+result = '';
+for (var i = 0; i < invalidRules.length; ++i) {
+    var itemText = invalidRules.item(i).cssText;
+    result += itemText + '\n';
+}
+document.getElementById('invalidResult').innerText = result;
+</script>

Modified: trunk/Source/WebCore/ChangeLog (102312 => 102313)


--- trunk/Source/WebCore/ChangeLog	2011-12-08 05:23:42 UTC (rev 102312)
+++ trunk/Source/WebCore/ChangeLog	2011-12-08 05:38:26 UTC (rev 102313)
@@ -1,3 +1,18 @@
+2011-12-07  Kenichi Ishibashi  <ba...@chromium.org>
+
+        Refactor CSSParser::parseFontFaceSrc()
+        https://bugs.webkit.org/show_bug.cgi?id=73989
+
+        Reviewed by Darin Adler.
+
+        Test: fast/css/font-face-src-parsing.html
+
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::parseFontFaceSrcURI): Added.
+        (WebCore::CSSParser::parseFontFaceSrcLocal): Added.
+        (WebCore::CSSParser::parseFontFaceSrc): Rewrote.
+        * css/CSSParser.h:
+
 2011-12-07  Xingnan Wang  <xingnan.w...@intel.com>
 
         Implement the SSE optimization in SincResampler::process()

Modified: trunk/Source/WebCore/css/CSSParser.cpp (102312 => 102313)


--- trunk/Source/WebCore/css/CSSParser.cpp	2011-12-08 05:23:42 UTC (rev 102312)
+++ trunk/Source/WebCore/css/CSSParser.cpp	2011-12-08 05:38:26 UTC (rev 102313)
@@ -4364,41 +4364,63 @@
     return false;
 }
 
-static bool isValidFormatFunction(CSSParserValue* val)
+bool CSSParser::parseFontFaceSrcURI(CSSValueList* valueList)
 {
-    CSSParserValueList* args = val->function->args.get();
-    return equalIgnoringCase(val->function->name, "format(") && (args->current()->unit == CSSPrimitiveValue::CSS_STRING || args->current()->unit == CSSPrimitiveValue::CSS_IDENT);
+    // FIXME: The completeURL call should be done when using the CSSFontFaceSrcValue,
+    // not when creating it.
+    RefPtr<CSSFontFaceSrcValue> uriValue(CSSFontFaceSrcValue::create(m_styleSheet->completeURL(m_valueList->current()->string)));
+
+    CSSParserValue* value = m_valueList->next();
+    if (!value) {
+        valueList->append(uriValue.release());
+        return true;
+    }
+    if (value->unit == CSSParserValue::Operator && value->iValue == ',') {
+        m_valueList->next();
+        valueList->append(uriValue.release());
+        return true;
+    }
+
+    if (value->unit != CSSParserValue::Function || !equalIgnoringCase(value->function->name, "format("))
+        return false;
+
+    // FIXME: http://www.w3.org/TR/2011/WD-css3-fonts-20111004/ says that format() contains a comma-separated list of strings,
+    // but CSSFontFaceSrcValue stores only one format. Allowing one format for now.
+    CSSParserValueList* args = value->function->args.get();
+    if (!args || args->size() != 1 || (args->current()->unit != CSSPrimitiveValue::CSS_STRING && args->current()->unit != CSSPrimitiveValue::CSS_IDENT))
+        return false;
+    uriValue->setFormat(args->current()->string);
+    valueList->append(uriValue.release());
+    value = m_valueList->next();
+    if (value && value->unit == CSSParserValue::Operator && value->iValue == ',')
+        m_valueList->next();
+    return true;
 }
 
-static bool parseFontFaceSrcFunction(CSSParserValue* value, bool& expectComma, bool& allowFormat, RefPtr<CSSFontFaceSrcValue>& uriValue, RefPtr<CSSFontFaceSrcValue>& parsedValue)
+bool CSSParser::parseFontFaceSrcLocal(CSSValueList* valueList)
 {
-    CSSParserValueList* args = value->function->args.get();
+    CSSParserValueList* args = m_valueList->current()->function->args.get();
     if (!args || !args->size())
         return false;
 
-    if (equalIgnoringCase(value->function->name, "local(") && !expectComma) {
-        expectComma = true;
-        allowFormat = false;
-        uriValue.clear();
-        if (args->current()->unit == CSSPrimitiveValue::CSS_STRING)
-            parsedValue = CSSFontFaceSrcValue::createLocal(args->current()->string);
-        else if (args->current()->unit == CSSPrimitiveValue::CSS_IDENT) {
-            StringBuilder builder;
-            for (CSSParserValue* localValue = args->current(); localValue; localValue = args->next()) {
-                if (localValue->unit != CSSPrimitiveValue::CSS_IDENT)
-                    return false;
-                if (!builder.isEmpty())
-                    builder.append(' ');
-                builder.append(localValue->string);
-            }
-            parsedValue = CSSFontFaceSrcValue::createLocal(builder.toString());
+    if (args->size() == 1 && args->current()->unit == CSSPrimitiveValue::CSS_STRING)
+        valueList->append(CSSFontFaceSrcValue::createLocal(args->current()->string));
+    else if (args->current()->unit == CSSPrimitiveValue::CSS_IDENT) {
+        StringBuilder builder;
+        for (CSSParserValue* localValue = args->current(); localValue; localValue = args->next()) {
+            if (localValue->unit != CSSPrimitiveValue::CSS_IDENT)
+                return false;
+            if (!builder.isEmpty())
+                builder.append(' ');
+            builder.append(localValue->string);
         }
-    } else if (args->size() == 1 && allowFormat && uriValue && isValidFormatFunction(value)) {
-        expectComma = true;
-        allowFormat = false;
-        uriValue->setFormat(args->current()->string);
-        uriValue.clear();
-        parsedValue.clear();
+        valueList->append(CSSFontFaceSrcValue::createLocal(builder.toString()));
+    } else
+        return false;
+
+    if (CSSParserValue* value = m_valueList->next()) {
+        if (value->unit == CSSParserValue::Operator && value->iValue == ',')
+            m_valueList->next();
     }
     return true;
 }
@@ -4406,53 +4428,23 @@
 bool CSSParser::parseFontFaceSrc()
 {
     RefPtr<CSSValueList> values(CSSValueList::createCommaSeparated());
-    CSSParserValue* val;
-    bool expectComma = false;
-    bool allowFormat = false;
-    bool failed = false;
-    RefPtr<CSSFontFaceSrcValue> uriValue;
-    while ((val = m_valueList->current())) {
-        RefPtr<CSSFontFaceSrcValue> parsedValue;
-        if (val->unit == CSSPrimitiveValue::CSS_URI && !expectComma && m_styleSheet) {
-            // FIXME: The completeURL call should be done when using the CSSFontFaceSrcValue,
-            // not when creating it.
-            parsedValue = CSSFontFaceSrcValue::create(m_styleSheet->completeURL(val->string));
-            uriValue = parsedValue;
-            allowFormat = true;
-            expectComma = true;
-        } else if (val->unit == CSSParserValue::Function) {
-            if (!parseFontFaceSrcFunction(val, expectComma, allowFormat, uriValue, parsedValue)) {
-                failed = true;
-                break;
-            }
-            if (parsedValue)
-                values->append(parsedValue.release());
-            m_valueList->next();
-            continue;
-        } else if (val->unit == CSSParserValue::Operator && val->iValue == ',' && expectComma) {
-            expectComma = false;
-            allowFormat = false;
-            uriValue.clear();
-            m_valueList->next();
-            continue;
-        }
 
-        if (parsedValue)
-            values->append(parsedValue.release());
-        else {
-            failed = true;
-            break;
-        }
-        m_valueList->next();
+    while (CSSParserValue* value = m_valueList->current()) {
+        if (value->unit == CSSPrimitiveValue::CSS_URI && m_styleSheet) {
+            if (!parseFontFaceSrcURI(values.get()))
+                return false;
+        } else if (value->unit == CSSParserValue::Function && equalIgnoringCase(value->function->name, "local(")) {
+            if (!parseFontFaceSrcLocal(values.get()))
+                return false;
+        } else
+            return false;
     }
+    if (!values->length())
+        return false;
 
-    if (values->length() && !failed) {
-        addProperty(CSSPropertySrc, values.release(), m_important);
-        m_valueList->next();
-        return true;
-    }
-
-    return false;
+    addProperty(CSSPropertySrc, values.release(), m_important);
+    m_valueList->next();
+    return true;
 }
 
 bool CSSParser::parseFontFaceUnicodeRange()

Modified: trunk/Source/WebCore/css/CSSParser.h (102312 => 102313)


--- trunk/Source/WebCore/css/CSSParser.h	2011-12-08 05:23:42 UTC (rev 102312)
+++ trunk/Source/WebCore/css/CSSParser.h	2011-12-08 05:38:26 UTC (rev 102313)
@@ -345,6 +345,9 @@
     bool parseSize(int propId, bool important);
     SizeParameterType parseSizeParameter(CSSValueList* parsedValues, CSSParserValue* value, SizeParameterType prevParamType);
 
+    bool parseFontFaceSrcURI(CSSValueList*);
+    bool parseFontFaceSrcLocal(CSSValueList*);
+
     UChar* m_data;
     UChar* yytext;
     UChar* yy_c_buf_p;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to