Skip to content

Commit

Permalink
Report bad prop in declaration w/ vendor/hack flag
Browse files Browse the repository at this point in the history 
This change causes errors to be reported for bad property names even in cases
where the declaration containing the property has an expression with values
that have been flagged as vendor extensions or CSS hacks.

Without this change, if an expression is flagged as containing a value that is a
vendor extension or CSS hack but the declaration it’s in has an invalid property
name (e.g., `background-image` misspelled as `backround-image` but with a value
of `-webkit-image-set(...)`), no error is emitted for the invalid property name.

This change works by 1) removing from the createProperty method in the
CssPropertyFactory code the warning-report handling for declarations that have
vendor extensions and CSS hacks, 2) moving that handling into the parser code,
where it now checks what method threw an InvalidParamException if one is thrown
when parsing a declaration, and 3) reporting an error if the method that threw
InvalidParamException in a vendor-extension/CSS-hack case is the createProperty
method (which means it was thrown for a bad property name) rather than other
code that throws it (which indicates that it’s a vendor-extension/CSS-hack
*value* rather than a bad property name).

(Incidentally, a potential alternative way of dealing with this problem would be
to not throw an InvalidParamException for bad property names in the
createProperty method to begin with, but to instead throw some different (new?)
exception that could be caught and handled separately.)

Note that this change also adds handling to emit a specific warning or error in
the case where a function name is a vendor extension (that is, the function name
starts with `-`) — because after the rest of this change is made to the code, no
error or warning would otherwise be reported in that case.
  • Loading branch information
@sideshowbarker
sideshowbarker committed Feb 15, 2018
1 parent d4f15aa commit f8c809a
Show file tree
Hide file tree
Showing 3 changed files with 148 additions and 126 deletions.
8 changes: 0 additions & 8 deletions org/w3c/css/parser/CssPropertyFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,16 +178,8 @@ public synchronized CssProperty createProperty(ApplContext ac, AtRule atRule, St
if (isVendorExtension(property)) {
throw new WarningParamException("vendor-extension", property);
}
if (expression.hasVendorExtensions()) {
throw new WarningParamException("vendor-extension", expression.toStringFromStart());
}
}

if (ac.getTreatCssHacksAsWarnings()) {
if (expression.hasCssHack()) {
throw new WarningParamException("css-hack", expression.toStringFromStart());
}
}
try {
atRuleMedia = (AtRuleMedia) atRule;
// TODO FIXME in fact, it should use a vector of media instead of extracting
Expand Down
249 changes: 132 additions & 117 deletions org/w3c/css/parser/analyzer/CssParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4137,7 +4137,14 @@ final public ArrayList<CssProperty> declarations() throws ParseException {if(!va
warningException.getMessageArgs());
} else {
values.starts();
addError(e, (CssExpression) values);
if (values.hasCssHack() || values.hasVendorExtensions()) {
if ("createProperty"
.equals(e.getStackTrace()[0].getMethodName())) {
addError(e, (CssExpression) values);
}
} else {
addError(e, (CssExpression) values);
}
}
}
{if ("" != null) return null;}
Expand Down Expand Up @@ -5390,6 +5397,14 @@ final public boolean prio() throws ParseException {
f.set(n.image.substring(0, n.image.length() - 1), exp);
if (funcname.charAt(0) == '-') {
exp.markVendorExtension();
if (ac.getTreatVendorExtensionsAsWarnings()) {
ac.getFrame().addWarning("vendor-extension",
funcname.substring(0, funcname.length() - 1));
} else {
addError(new ParseException(ac.getMsg()
.getString("warning.vendor-extension")),
funcname);
}
}
{if ("" != null) return f;}
}
Expand Down Expand Up @@ -5731,36 +5746,12 @@ private boolean jj_2_5(int xla)
finally { jj_save(4, xla); }
}

private boolean jj_3R_175()
{
if (jj_scan_token(STRING)) return true;
return false;
}

private boolean jj_3R_154()
{
if (jj_scan_token(PLUS)) return true;
return false;
}

private boolean jj_3R_174()
{
if (jj_3R_183()) return true;
return false;
}

private boolean jj_3R_173()
{
if (jj_3R_152()) return true;
return false;
}

private boolean jj_3R_172()
{
if (jj_3R_151()) return true;
return false;
}

private boolean jj_3R_153()
{
if (jj_scan_token(MINUS)) return true;
Expand All @@ -5778,47 +5769,6 @@ private boolean jj_3R_137()
return false;
}

private boolean jj_3R_139()
{
Token xsp;
xsp = jj_scanpos;
if (jj_3R_172()) {
jj_scanpos = xsp;
if (jj_3R_173()) {
jj_scanpos = xsp;
if (jj_3R_174()) {
jj_scanpos = xsp;
if (jj_3R_175()) {
jj_scanpos = xsp;
if (jj_3R_176()) {
jj_scanpos = xsp;
if (jj_3R_177()) {
jj_scanpos = xsp;
if (jj_3R_178()) {
jj_scanpos = xsp;
if (jj_3R_179()) {
jj_scanpos = xsp;
if (jj_3R_180()) {
jj_scanpos = xsp;
if (jj_3R_181()) return true;
}
}
}
}
}
}
}
}
}
return false;
}

private boolean jj_3R_128()
{
if (jj_scan_token(S)) return true;
return false;
}

private boolean jj_3R_124()
{
Token xsp;
Expand Down Expand Up @@ -5861,20 +5811,6 @@ private boolean jj_3R_169()
return false;
}

private boolean jj_3R_182()
{
if (jj_scan_token(COMMA)) return true;
return false;
}

private boolean jj_3R_140()
{
Token xsp;
xsp = jj_scanpos;
if (jj_3R_182()) jj_scanpos = xsp;
return false;
}

private boolean jj_3_3()
{
Token xsp;
Expand Down Expand Up @@ -5931,6 +5867,12 @@ private boolean jj_3_2()
return false;
}

private boolean jj_3R_182()
{
if (jj_scan_token(COMMA)) return true;
return false;
}

private boolean jj_3R_166()
{
if (jj_scan_token(ST)) return true;
Expand All @@ -5949,6 +5891,14 @@ private boolean jj_3R_164()
return false;
}

private boolean jj_3R_140()
{
Token xsp;
xsp = jj_scanpos;
if (jj_3R_182()) jj_scanpos = xsp;
return false;
}

private boolean jj_3R_163()
{
if (jj_scan_token(TIME)) return true;
Expand Down Expand Up @@ -6015,6 +5965,17 @@ private boolean jj_3R_142()
return false;
}

private boolean jj_3R_184()
{
Token xsp;
xsp = jj_scanpos;
if (jj_scan_token(37)) {
jj_scanpos = xsp;
if (jj_scan_token(38)) return true;
}
return false;
}

private boolean jj_3R_138()
{
Token xsp;
Expand Down Expand Up @@ -6085,28 +6046,6 @@ private boolean jj_3R_131()
return false;
}

private boolean jj_3R_123()
{
Token xsp;
xsp = jj_scanpos;
if (jj_scan_token(36)) {
jj_scanpos = xsp;
if (jj_scan_token(49)) return true;
}
return false;
}

private boolean jj_3R_184()
{
Token xsp;
xsp = jj_scanpos;
if (jj_scan_token(37)) {
jj_scanpos = xsp;
if (jj_scan_token(38)) return true;
}
return false;
}

private boolean jj_3R_152()
{
if (jj_scan_token(FUNCTIONATTR)) return true;
Expand Down Expand Up @@ -6146,6 +6085,23 @@ private boolean jj_3R_130()
return false;
}

private boolean jj_3R_123()
{
Token xsp;
xsp = jj_scanpos;
if (jj_scan_token(36)) {
jj_scanpos = xsp;
if (jj_scan_token(49)) return true;
}
return false;
}

private boolean jj_3R_186()
{
if (jj_scan_token(IDENT)) return true;
return false;
}

private boolean jj_3R_151()
{
if (jj_scan_token(FUNCTIONCALC)) return true;
Expand Down Expand Up @@ -6189,27 +6145,12 @@ private boolean jj_3R_135()
return false;
}

private boolean jj_3_1()
{
Token xsp;
xsp = jj_scanpos;
if (jj_3R_123()) jj_scanpos = xsp;
if (jj_scan_token(108)) return true;
return false;
}

private boolean jj_3R_134()
{
if (jj_scan_token(RPARAN)) return true;
return false;
}

private boolean jj_3R_186()
{
if (jj_scan_token(IDENT)) return true;
return false;
}

private boolean jj_3R_150()
{
if (jj_scan_token(FREQ)) return true;
Expand Down Expand Up @@ -6240,6 +6181,15 @@ private boolean jj_3R_146()
return false;
}

private boolean jj_3_1()
{
Token xsp;
xsp = jj_scanpos;
if (jj_3R_123()) jj_scanpos = xsp;
if (jj_scan_token(108)) return true;
return false;
}

private boolean jj_3R_145()
{
if (jj_scan_token(RELVIEWLENGTH)) return true;
Expand Down Expand Up @@ -6375,6 +6325,71 @@ private boolean jj_3R_176()
return false;
}

private boolean jj_3R_175()
{
if (jj_scan_token(STRING)) return true;
return false;
}

private boolean jj_3R_174()
{
if (jj_3R_183()) return true;
return false;
}

private boolean jj_3R_173()
{
if (jj_3R_152()) return true;
return false;
}

private boolean jj_3R_172()
{
if (jj_3R_151()) return true;
return false;
}

private boolean jj_3R_139()
{
Token xsp;
xsp = jj_scanpos;
if (jj_3R_172()) {
jj_scanpos = xsp;
if (jj_3R_173()) {
jj_scanpos = xsp;
if (jj_3R_174()) {
jj_scanpos = xsp;
if (jj_3R_175()) {
jj_scanpos = xsp;
if (jj_3R_176()) {
jj_scanpos = xsp;
if (jj_3R_177()) {
jj_scanpos = xsp;
if (jj_3R_178()) {
jj_scanpos = xsp;
if (jj_3R_179()) {
jj_scanpos = xsp;
if (jj_3R_180()) {
jj_scanpos = xsp;
if (jj_3R_181()) return true;
}
}
}
}
}
}
}
}
}
return false;
}

private boolean jj_3R_128()
{
if (jj_scan_token(S)) return true;
return false;
}

/** Generated Token Manager. */
public CssParserTokenManager token_source;
SimpleCharStream jj_input_stream;
Expand Down
17 changes: 16 additions & 1 deletion org/w3c/css/parser/analyzer/CssParser.jj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2415,7 +2415,14 @@ try {
warningException.getMessageArgs());
} else {
values.starts();
addError(e, (CssExpression) values);
if (values.hasCssHack() || values.hasVendorExtensions()) {
if ("createProperty"
.equals(e.getStackTrace()[0].getMethodName())) {
addError(e, (CssExpression) values);
}
} else {
addError(e, (CssExpression) values);
}
}
}
return null;
Expand Down Expand Up @@ -2852,6 +2859,14 @@ CssValue function() :
f.set(n.image.substring(0, n.image.length() - 1), exp);
if (funcname.charAt(0) == '-') {
exp.markVendorExtension();
if (ac.getTreatVendorExtensionsAsWarnings()) {
ac.getFrame().addWarning("vendor-extension",
funcname.substring(0, funcname.length() - 1));
} else {
addError(new ParseException(ac.getMsg()
.getString("warning.vendor-extension")),
funcname);
}
}
return f;
}
Expand Down

0 comments on commit f8c809a

Please sign in to comment.