Make css-defs.js and the display property definition vendor-prefix agnostic

I updated the "display" property which included several vendor-prefixed values
(-moz-inline-stack and -moz-inline-box) so that they work with sanitizecss.js
which now strips the prefix before checking the white-list, and I changed
CssPropertyPatterns to strip prefixes when generating white-lists.

[MikeSamuel, 2013-09-03 16:19:47]

I updated the "display" property which included several vendor-prefixed values
(-moz-inline-stack and -moz-inline-box) so that they work with sanitizecss.js
which now strips the prefix before checking the white-list, and I changed
CssPropertyPatterns to strip prefixes when generating white-lists.

[MikeSamuel, 2013-09-03 16:20:49]

I updated the "display" property which included several vendor-prefixed values
(-moz-inline-stack and -moz-inline-box) so that they work with sanitizecss.js
which now strips the prefix before checking the white-list, and I changed
CssPropertyPatterns to strip prefixes when generating white-lists.

[MikeSamuel, 2013-09-03 16:21:20]

Ping

2013/8/7  <reply@codereview-hr.appspotmail.com>:
> Reviewers: felix8a,
>
> Description:
> I updated the "display" property which included several vendor-prefixed
> values
> (-moz-inline-stack and -moz-inline-box) so that they work with
> sanitizecss.js
> which now strips the prefix before checking the white-list, and I
> changed
> CssPropertyPatterns to strip prefixes when generating white-lists.
>
> Please review this at https://codereview.appspot.com/12625043/
>
> Affected files:
>   M     src/com/google/caja/lang/css/CssPropertyPatterns.java
>   M     src/com/google/caja/lang/css/css-extensions-defs.json
>   M     src/com/google/caja/lang/css/css-extensions-whitelist.json
>
>
> Index: src/com/google/caja/lang/css/CssPropertyPatterns.java
> ===================================================================
> --- src/com/google/caja/lang/css/CssPropertyPatterns.java       (revision
> 5535)
> +++ src/com/google/caja/lang/css/CssPropertyPatterns.java       (working
> copy)
> @@ -158,6 +158,38 @@
>      return data;
>    }
>
> +  private static final Set<String> KNOWN_VENDOR_PREFIXES =
> Sets.immutableSet(
> +      "apple",
> +      "css",
> +      "epub",
> +      "khtml",
> +      "moz",
> +      "ms",
> +      "mso",
> +      "o",
> +      "rim",
> +      "wap",
> +      "webkit",
> +      "xv"
> +  );
> +
> +  public static String withoutVendorPrefix(String cssIdentifier) {
> +    if (cssIdentifier.startsWith("-")) {
> +      int dash = cssIdentifier.indexOf('-', 1);
> +      if (dash >= 0) {
> +        String possiblePrefix = cssIdentifier.substring(1, dash);
> +        if (KNOWN_VENDOR_PREFIXES.contains(possiblePrefix)) {
> +          return cssIdentifier.substring(dash + 1);
> +        }
> +      }
> +    }
> +    return cssIdentifier;
> +  }
> +
> +  public static boolean hasVendorPrefix(String cssIdentifier) {
> +    return !cssIdentifier.equals(withoutVendorPrefix(cssIdentifier));
> +  }
> +
>    /**
>     * Walks a property signature to figure out what tokens can comprise its
>     * value and how non-symbolic tokens like quoted strings, and non-keyword
> @@ -215,7 +247,7 @@
>        // all positive matches are followed by token-breaking space.
>        // The pattern as a whole can then be matched against the value with
> one
>        // space added at the end.
> -      data.literals.add(litValue);
> +      data.literals.add(withoutVendorPrefix(litValue));
>      }
>
>      private void inspectRep(CssPropertySignature.RepeatedSignature sig) {
> @@ -327,6 +359,7 @@
>      for (CssSchema.CssPropertyInfo prop : props) {
>        if (!schema.isPropertyAllowed(prop.name)) { continue; }
>        String key = prop.name.getCanonicalForm();
> +      if (hasVendorPrefix(key)) { continue; }
>        CssPropertyData data = new CssPropertyData(key, prop.sig);
>        pp.new Inspector(data).inspect();
>        propData.add(Pair.pair(prop, data));
> Index: src/com/google/caja/lang/css/css-extensions-defs.json
> ===================================================================
> --- src/com/google/caja/lang/css/css-extensions-defs.json       (revision
> 5535)
> +++ src/com/google/caja/lang/css/css-extensions-defs.json       (working
> copy)
> @@ -298,13 +298,60 @@
>        "source": "http://www.quirksmode.org/css/cursor.html" },
>
>      { "key": "display",
> -      "signature": "inline | block | list-item | run-in | inline-block |
> table | inline-table | table-row-group | table-header-group |
> table-footer-group | table-row | table-column-group | table-column |
> table-cell | table-caption | none | inherit | -moz-inline-box |
> -moz-inline-stack",
> +      "signature": "inline | block | list-item | run-in | inline-list-item
> | inline-block | table | inline-table | table-cell | table-caption | flex |
> inline-flex | grid | inline-grid | [ <display-inside> || <display-outside>
> || <display-box> || <display-extras> ] | inherit | inline-box |
> inline-stack",
>        "default": "inline",
>        "appliesTo": "*",
>        "inherited": false,
>        "mediaGroups": "*",
> -      "dom2property": "display" },
> +      "dom2property": "display",
> +      "source": "http://dev.w3.org/csswg/css-display-3/#the-display",
> +      "comment": "inline-box and inline-stack are not in the spec but are
> recognized" },
>
> +    { "key": "display-inside",
> +      "signature": "<display-inside>",
> +      "default": "auto",
> +      "appliesTo": "*",
> +      "inherited": false,
> +      "mediaGroups": "*",
> +      "source": "http://dev.w3.org/csswg/css-display-3/#display-inside" },
> +
> +    { "key": "<display-inside>",
> +      "signature": "auto | block | table | flex | grid" },
> +
> +    { "key": "display-outside",
> +      "signature": "<display-outside>",
> +      "default": "inline-level",
> +      "appliesTo": "*",
> +      "inherited": false,
> +      "mediaGroups": "*",
> +      "source": "http://dev.w3.org/csswg/css-display-3/#display-outside" },
> +
> +    { "key": "<display-outside>",
> +      "signature": "block-level | inline-level | none | table-row-group |
> table-header-group | table-footer-group | table-row | table-cell |
> table-column-group | table-column | table-caption" },
> +
> +    { "key": "<display-box>",
> +      "signature": "normal | none | contents",
> +      "comment": "not defined due to possible spec error.
> http://lists.w3.org/Archives/Public/www-style/2012Nov/0459.html suggests it
> is similar to box" },
> +
> +    { "key": "display-extras",
> +      "signature": "<display-extras>",
> +      "default": "inline-level",
> +      "appliesTo": "*",
> +      "inherited": false,
> +      "mediaGroups": "*",
> +      "source": "http://dev.w3.org/csswg/css-display-3/#display-extras" },
> +
> +    { "key": "<display-extras>",
> +      "signature": "none | [ list-item ]" },
> +
> +    { "key": "box",
> +      "signature": "normal | none | contents",
> +      "default": "normal",
> +      "appliesTo": "*",
> +      "inherited": false,
> +      "mediaGroups": "*",
> +      "source": "http://dev.w3.org/csswg/css-display-3/#box" },
> +
>      { "key": "filter",
>        "signature": "[ <ie-filter-opacity> | <prog-id> ]+",
>        "default": null,
> Index: src/com/google/caja/lang/css/css-extensions-whitelist.json
> ===================================================================
> --- src/com/google/caja/lang/css/css-extensions-whitelist.json  (revision
> 5535)
> +++ src/com/google/caja/lang/css/css-extensions-whitelist.json  (working
> copy)
> @@ -19,8 +19,12 @@
>      "border-radius",
>      "border-top-left-radius",
>      "border-top-right-radius",
> +    "box",
>      "box-shadow",
>      "content",
> +    "display-extras",
> +    "display-inside",
> +    "display-outside",
>      "filter",
>      "font-stretch",
>      "-moz-border-radius",
>
>

[felix8a, 2013-09-03 19:01:16]

lgtm