Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(113)

Issue 5836053: Integrate CSS rewriter tests with client side CSS rewriter (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 3 months ago by MikeSamuel
Modified:
14 years, 3 months ago
Reviewers:
Jasvir
CC:
google-caja-discuss_googlegroups.com
Base URL:
http://google-caja.googlecode.com/svn/trunk/
Visibility:
Public.

Description

This fixes a number of bugs in the client side CSS rewriter and wires up tests. Following CLs will: 1. add another parameter to sanitizeStylesheet to take a URL policy and re-enable the background-image tests for the client side rewriter 2. remove trivial differences in the output between the two rewriters to deal with ordering of split rulesets, choice of quotes, whether font-names are case-preserved 3. implement case-sensitive rule bodies Submitted @4809

Patch Set 1 #

Patch Set 2 : Integrate CSS rewriter tests with client side CSS rewriter #

Patch Set 3 : Integrate CSS rewriter tests with client side CSS rewriter #

Patch Set 4 : Integrate CSS rewriter tests with client side CSS rewriter #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+247 lines, -78 lines) Patch
M src/com/google/caja/plugin/cssparser.js View 1 2 3 1 chunk +4 lines, -3 lines 0 comments Download
M src/com/google/caja/plugin/sanitizecss.js View 5 chunks +17 lines, -13 lines 0 comments Download
M tests/com/google/caja/plugin/CssRewriterTest.java View 1 2 chunks +6 lines, -1 line 0 comments Download
A tests/com/google/caja/plugin/CssStylesheetTest.java View 1 chunk +30 lines, -0 lines 0 comments Download
A tests/com/google/caja/plugin/css-stylesheet-test.html View 1 chunk +33 lines, -0 lines 0 comments Download
A tests/com/google/caja/plugin/css-stylesheet-test.js View 1 chunk +48 lines, -0 lines 0 comments Download
M tests/com/google/caja/plugin/css-stylesheet-tests.js View 1 2 24 chunks +109 lines, -61 lines 1 comment Download

Messages

Total messages: 3
MikeSamuel
14 years, 3 months ago (2012-03-15 22:26:54 UTC) #1
Jasvir
LGTM http://codereview.appspot.com/5836053/diff/5001/tests/com/google/caja/plugin/css-stylesheet-tests.js File tests/com/google/caja/plugin/css-stylesheet-tests.js (right): http://codereview.appspot.com/5836053/diff/5001/tests/com/google/caja/plugin/css-stylesheet-tests.js#newcode144 tests/com/google/caja/plugin/css-stylesheet-tests.js:144: "altGolden": 'a{font:12pt "times new roman" , "times" ,' ...
14 years, 3 months ago (2012-03-16 05:06:10 UTC) #2
MikeSamuel
14 years, 3 months ago (2012-03-16 05:32:09 UTC) #3
On 2012/03/16 05:06:10, Jasvir wrote:
> LGTM
> 
>
http://codereview.appspot.com/5836053/diff/5001/tests/com/google/caja/plugin/...
> File tests/com/google/caja/plugin/css-stylesheet-tests.js (right):
> 
>
http://codereview.appspot.com/5836053/diff/5001/tests/com/google/caja/plugin/...
> tests/com/google/caja/plugin/css-stylesheet-tests.js:144: "altGolden":
> 'a{font:12pt "times new roman" , "times" ,'
> Aren't font names case sensitive?

Hrmm.  http://www.w3.org/TR/css3-syntax/ says

> The following rules always hold:
All CSS style sheets are case-insensitive, except for parts that are not under
the control of CSS. For example, the case-sensitivity of values of the HTML
attributes "id" and "class", of font names, and of URIs lies outside the scope
of this specification. Note in particular that element names are
case-insensitive in HTML, but case-sensitive in XML.

http://www.w3.org/TR/css3-fonts/ contradicts that

> For other family names, the user agent attempts to find the family name among
fonts defined via @font-face rules and then among available system fonts,
matching names with a case-insensitive comparison.

and that is specifically for font-families which seems controlling.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b