Attachment 214743 Details for Bug 123082 – patch. incorporates the feedback: There should be a space between WHITESPACE and |

[patch] patch. incorporates the feedback: There should be a space between WHITESPACE and |

patch123082WHITESPACECSSGrammar.diff (text/plain), 11.41 KB, created by Mihai Maerean on 2013-10-21 09:02:20 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Mihai Maerean

Created: 2013-10-21 09:02:20 PDT

Size: 11.41 KB

patch

obsolete

>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 6d9c59c..8049627 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,19 @@
>+2013-10-20  Mihai Maerean  <mmaerean@adobe.com>
>+
>+        [CSS Regions] Fix WHITESPACE issues in the CSS grammar.
>+        https://bugs.webkit.org/show_bug.cgi?id=123082
>+
>+        Reviewed by Andreas Kling.
>+
>+        This is a port of Rune Lillesveen's patch from https://codereview.chromium.org/25607005
>+
>+        * css3/filters/custom-with-at-rule-syntax/parsing-at-rule-valid-expected.txt:
>+        * css3/filters/custom-with-at-rule-syntax/script-tests/parsing-at-rule-valid.js:
>+        * fast/css/calc-comments-allowed-expected.html: Added.
>+        * fast/css/calc-comments-allowed.html: Added.
>+        * fast/regions/webkit-region-syntax-space-expected.txt: Added.
>+        * fast/regions/webkit-region-syntax-space.html: Added.
>+
> 2013-10-21  Santosh Mahto  <santosh.ma@samsung.com>
> 
>         ASSERTION FAILED: !style->propertyIsImportant(propertyID) in WebCore::setTextDecorationProperty
>diff --git a/LayoutTests/css3/filters/custom-with-at-rule-syntax/parsing-at-rule-valid-expected.txt b/LayoutTests/css3/filters/custom-with-at-rule-syntax/parsing-at-rule-valid-expected.txt
>index 8d1838d..048ddda 100644
>--- a/LayoutTests/css3/filters/custom-with-at-rule-syntax/parsing-at-rule-valid-expected.txt
>+++ b/LayoutTests/css3/filters/custom-with-at-rule-syntax/parsing-at-rule-valid-expected.txt
>@@ -46,6 +46,42 @@ PASS declaration.getPropertyValue('width') is "100px"
> PASS declaration.getPropertyValue('height') is "100px"
> PASS declaration.length is 2
> 
>+Empty rule, comment separators
>+@-webkit-filter/**/my-filter/**/{ }
>+PASS cssRule.cssText is "@-webkit-filter my-filter { }"
>+PASS cssRule.type is CSSRule.WEBKIT_FILTER_RULE
>+PASS cssRule instanceof WebKitCSSFilterRule is true
>+PASS cssRule.constructor === WebKitCSSFilterRule is true
>+PASS cssRule.__proto__ === WebKitCSSFilterRule.prototype is true
>+PASS declaration.length is 0
>+
>+Empty rule, comment+space separators
>+@-webkit-filter/**/ my-filter/**/ { }
>+PASS cssRule.cssText is "@-webkit-filter my-filter { }"
>+PASS cssRule.type is CSSRule.WEBKIT_FILTER_RULE
>+PASS cssRule instanceof WebKitCSSFilterRule is true
>+PASS cssRule.constructor === WebKitCSSFilterRule is true
>+PASS cssRule.__proto__ === WebKitCSSFilterRule.prototype is true
>+PASS declaration.length is 0
>+
>+Empty rule, space+comment separators
>+@-webkit-filter /**/my-filter /**/{ }
>+PASS cssRule.cssText is "@-webkit-filter my-filter { }"
>+PASS cssRule.type is CSSRule.WEBKIT_FILTER_RULE
>+PASS cssRule instanceof WebKitCSSFilterRule is true
>+PASS cssRule.constructor === WebKitCSSFilterRule is true
>+PASS cssRule.__proto__ === WebKitCSSFilterRule.prototype is true
>+PASS declaration.length is 0
>+
>+Empty rule, space+comment+space separators
>+@-webkit-filter /**/ my-filter /**/ { }
>+PASS cssRule.cssText is "@-webkit-filter my-filter { }"
>+PASS cssRule.type is CSSRule.WEBKIT_FILTER_RULE
>+PASS cssRule instanceof WebKitCSSFilterRule is true
>+PASS cssRule.constructor === WebKitCSSFilterRule is true
>+PASS cssRule.__proto__ === WebKitCSSFilterRule.prototype is true
>+PASS declaration.length is 0
>+
> ========================================
> Nested filter at-rule tests.
> ========================================
>diff --git a/LayoutTests/css3/filters/custom-with-at-rule-syntax/script-tests/parsing-at-rule-valid.js b/LayoutTests/css3/filters/custom-with-at-rule-syntax/script-tests/parsing-at-rule-valid.js
>index a02138a..18e5996 100644
>--- a/LayoutTests/css3/filters/custom-with-at-rule-syntax/script-tests/parsing-at-rule-valid.js
>+++ b/LayoutTests/css3/filters/custom-with-at-rule-syntax/script-tests/parsing-at-rule-valid.js
>@@ -70,6 +70,18 @@ testFilterAtRule("Rule with arbitrary properties.",
>     "@-webkit-filter my-filter { width: 100px; height: 100px; }", 
>     "@-webkit-filter my-filter { width: 100px; height: 100px; }",
>     {width: "100px", height: "100px"});
>+testFilterAtRule("Empty rule, comment separators",
>+    "@-webkit-filter/**/my-filter/**/{ }",
>+    "@-webkit-filter my-filter { }");
>+testFilterAtRule("Empty rule, comment+space separators",
>+    "@-webkit-filter/**/ my-filter/**/ { }",
>+    "@-webkit-filter my-filter { }");
>+testFilterAtRule("Empty rule, space+comment separators",
>+    "@-webkit-filter /**/my-filter /**/{ }",
>+    "@-webkit-filter my-filter { }");
>+testFilterAtRule("Empty rule, space+comment+space separators",
>+    "@-webkit-filter /**/ my-filter /**/ { }",
>+    "@-webkit-filter my-filter { }");
> 
> heading("Nested filter at-rule tests.");
> testNestedRules("Nested rule.",
>diff --git a/LayoutTests/fast/css/calc-comments-allowed-expected.html b/LayoutTests/fast/css/calc-comments-allowed-expected.html
>new file mode 100644
>index 0000000..50946fa
>--- /dev/null
>+++ b/LayoutTests/fast/css/calc-comments-allowed-expected.html
>@@ -0,0 +1,3 @@
>+<!DOCTYPE html>
>+<p>You should see a green rectangle below</p>
>+<div style="width:100px;height:250px;background-color:green"></div>
>diff --git a/LayoutTests/fast/css/calc-comments-allowed.html b/LayoutTests/fast/css/calc-comments-allowed.html
>new file mode 100644
>index 0000000..4d5c878
>--- /dev/null
>+++ b/LayoutTests/fast/css/calc-comments-allowed.html
>@@ -0,0 +1,22 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <title>Comments inside calc() allowed - http://wkb.ug/123082</title>
>+    <style>
>+        div { height: 50px; background-color: green; font-size: 50px; }
>+        #t1 { width: calc(50px + 1em) }
>+        #t2 { width: calc(50px + /* Comment allowed */ 1em) }
>+        #t3 { width: calc(150px - /* Comment allowed */ 1em) }
>+        #t4 { width: calc(50px * /* Comment allowed */ 2) }
>+        #t5 { width: calc(200px / /* Comment allowed */ 2) }
>+    </style>
>+</head>
>+<body>
>+    <p>You should see a green rectangle below</p>
>+    <div id="t1"></div>
>+    <div id="t2"></div>
>+    <div id="t3"></div>
>+    <div id="t4"></div>
>+    <div id="t5"></div>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/regions/webkit-region-syntax-space-expected.txt b/LayoutTests/fast/regions/webkit-region-syntax-space-expected.txt
>new file mode 100644
>index 0000000..28068f4
>--- /dev/null
>+++ b/LayoutTests/fast/regions/webkit-region-syntax-space-expected.txt
>@@ -0,0 +1,6 @@
>+
>+PASS Comment between @-symbol and flow name 
>+PASS Comment + space between @-symbol and flow name 
>+PASS Space + comment between @-symbol and flow name 
>+PASS Space + comment + space between @-symbol and flow name 
>+
>diff --git a/LayoutTests/fast/regions/webkit-region-syntax-space.html b/LayoutTests/fast/regions/webkit-region-syntax-space.html
>new file mode 100644
>index 0000000..1cdbf16
>--- /dev/null
>+++ b/LayoutTests/fast/regions/webkit-region-syntax-space.html
>@@ -0,0 +1,32 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <title>Spaces in @-webkit-region prelude - http://wkb.ug/123082</title>
>+    <script src="../../resources/testharness.js"></script>
>+    <script src="../../resources/testharnessreport.js"></script>
>+    <style>@-webkit-region/**/#flow { #id {} }</style>
>+    <style>@-webkit-region/**/ #flow { #id {} }</style>
>+    <style>@-webkit-region /**/#flow { #id {} }</style>
>+    <style>@-webkit-region /**/ #flow { #id {} }</style>
>+</head>
>+<body>
>+    <div id="log"></div>
>+    <script>
>+        test(function(){
>+            assert_equals(document.styleSheets[0].cssRules.length, 1, "Rule valid and in CSSOM.");
>+        }, "Comment between @-symbol and flow name");
>+
>+        test(function(){
>+            assert_equals(document.styleSheets[1].cssRules.length, 1, "Rule valid and in CSSOM.");
>+        }, "Comment + space between @-symbol and flow name");
>+
>+        test(function(){
>+            assert_equals(document.styleSheets[2].cssRules.length, 1, "Rule valid and in CSSOM.");
>+        }, "Space + comment between @-symbol and flow name");
>+
>+        test(function(){
>+            assert_equals(document.styleSheets[3].cssRules.length, 1, "Rule valid and in CSSOM.");
>+        }, "Space + comment + space between @-symbol and flow name");
>+    </script>
>+</body>
>+</html>
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 9896dff..46707a8 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2013-10-20  Mihai Maerean  <mmaerean@adobe.com>
>+
>+        [CSS Regions] Fix WHITESPACE issues in the CSS grammar.
>+        https://bugs.webkit.org/show_bug.cgi?id=123082
>+
>+        Reviewed by Andreas Kling.
>+
>+        This is a port of Rune Lillesveen's patch from https://codereview.chromium.org/25607005
>+
>+        Fix WHITESPACE issues in the CSS grammar.
>+
>+        A single WHITESPACE token consumes consecutive spaces, but does not consume
>+        spaces separated by comments. That means S* and S+ in CSS grammars need to
>+        accept multiple WHITESPACE tokens. Additionally, white spaces are not
>+        mandatory to separate an @-symbol and the rest of the prelude.
>+
>+        Use space non-terminal instead of WHITESPACE for S+ in calc expressions.
>+
>+        Use maybe_space non-terminal instead of WHITESPACE for S* after @-webkit-filter
>+        and @-webkit-region.
>+
>+        Tests: fast/css/calc-comments-allowed.html
>+               fast/regions/webkit-region-syntax-space.html
>+
>+        * css/CSSGrammar.y.in:
>+
> 2013-10-21  Santosh Mahto  <santosh.ma@samsung.com>
> 
>         ASSERTION FAILED: !style->propertyIsImportant(propertyID) in WebCore::setTextDecorationProperty
>diff --git a/Source/WebCore/css/CSSGrammar.y.in b/Source/WebCore/css/CSSGrammar.y.in
>index d693b2b..6da5d52 100644
>--- a/Source/WebCore/css/CSSGrammar.y.in
>+++ b/Source/WebCore/css/CSSGrammar.y.in
>@@ -369,6 +369,9 @@ webkit_supports_condition: WEBKIT_SUPPORTS_CONDITION_SYM '{' maybe_space support
> 
> #endif
> 
>+/* for expressions that require at least one whitespace to be present, like the + and - operators in calc expressions */
>+space: WHITESPACE | space WHITESPACE ;
>+
> maybe_space: /* empty */ %prec UNIMPORTANT_TOK | maybe_space WHITESPACE ;
> 
> maybe_sgml: /* empty */ | maybe_sgml SGML_CD | maybe_sgml WHITESPACE ;
>@@ -954,7 +957,7 @@ before_region_rule:
>     ;
> 
> region:
>-    before_region_rule WEBKIT_REGION_RULE_SYM WHITESPACE selector_list at_rule_header_end '{' at_rule_body_start maybe_space block_valid_rule_list save_block {
>+    before_region_rule WEBKIT_REGION_RULE_SYM maybe_space selector_list at_rule_header_end '{' at_rule_body_start maybe_space block_valid_rule_list save_block {
>         OwnPtr<Vector<RefPtr<StyleRuleBase>>> ruleList = adoptPtr($9);
>         if ($4)
>             $$ = parser->createRegionRule(adoptPtr($4).get(), ruleList.get()).leakRef();
>@@ -975,7 +978,7 @@ before_filter_rule:
>     ;
> 
> filter:
>-    before_filter_rule WEBKIT_FILTER_RULE_SYM WHITESPACE IDENT at_rule_header_end_maybe_space '{' at_rule_body_start maybe_space_before_declaration declaration_list closing_brace {
>+    before_filter_rule WEBKIT_FILTER_RULE_SYM maybe_space IDENT at_rule_header_end_maybe_space '{' at_rule_body_start maybe_space_before_declaration declaration_list closing_brace {
>         parser->m_inFilterRule = false;
>         $$ = parser->createFilterRule($4).leakRef();
>     }
>@@ -1583,11 +1586,16 @@ calc_func_term:
>   | unary_operator unary_term { $$ = $2; $$.fValue *= $1; }
>   ;
> 
>+/*
>+ * The grammar requires spaces around binary â+â and â-â operators.
>+ * The '*' and '/' operators do not require spaces.
>+ * http://www.w3.org/TR/css3-values/#calc-syntax
>+ */
> calc_func_operator:
>-    WHITESPACE '+' WHITESPACE {
>+    space '+' space {
>         $$ = '+';
>     }
>-    | WHITESPACE '-' WHITESPACE {
>+    | space '-' space {
>         $$ = '-';
>     }
>     | calc_maybe_space '*' maybe_space {

Actions: View | Formatted Diff | Diff

Attachments on bug 123082: 214713 | 214733 | 214739 | 214741 | 214743