Bug 1462553 - fix !important parsing in devtools; r?pbro
Bug 1462553 points out that the CSS-parsing code in parsing-utils was
not correctly handling "!important"; in particular, it was allowing
this to appear in the middle of a declaration, rather than only at the
end. This patch fixes the parser.
MozReview-Commit-ID: 9efv60gX6nV
--- a/devtools/client/shared/test/unit/test_parseDeclarations.js
+++ b/devtools/client/shared/test/unit/test_parseDeclarations.js
@@ -66,23 +66,70 @@ const TEST_DATA = [
// Test simple priority
{
input: "p1: v1 ! important; p2: v2 ! important;",
expected: [
{name: "p1", value: "v1", priority: "important", offsets: [0, 20]},
{name: "p2", value: "v2", priority: "important", offsets: [21, 40]}
]
},
+ // Test simple priority
+ {
+ input: "p1: v1 !/*comment*/important;",
+ expected: [
+ {name: "p1", value: "v1", priority: "important", offsets: [0, 29]},
+ ]
+ },
+ // Test priority without terminating ";".
+ {
+ input: "p1: v1 !important",
+ expected: [
+ {name: "p1", value: "v1", priority: "important", offsets: [0, 17]},
+ ]
+ },
+ // Test trailing "!" without terminating ";".
+ {
+ input: "p1: v1 !",
+ expected: [
+ {name: "p1", value: "v1 !", priority: "", offsets: [0, 8]},
+ ]
+ },
// Test invalid priority
{
input: "p1: v1 important;",
expected: [
{name: "p1", value: "v1 important", priority: "", offsets: [0, 17]}
]
},
+ // Test invalid priority (in the middle of the declaration).
+ // See bug 1462553.
+ {
+ input: "p1: v1 !important v2;",
+ expected: [
+ {name: "p1", value: "v1 !important v2", priority: "", offsets: [0, 21]}
+ ]
+ },
+ {
+ input: "p1: v1 ! important v2;",
+ expected: [
+ {name: "p1", value: "v1 ! important v2", priority: "", offsets: [0, 25]}
+ ]
+ },
+ {
+ input: "p1: v1 ! /*comment*/ important v2;",
+ expected: [
+ {name: "p1", value: "v1 ! important v2", priority: "", offsets: [0, 36]}
+ ]
+ },
+ {
+ input: "p1: v1 !/*hi*/important v2;",
+ expected: [
+ {name: "p1", value: "v1 ! important v2", priority: "", offsets: [0, 27]}
+ ]
+ },
// Test various types of background-image urls
{
input: "background-image: url(../../relative/image.png)",
expected: [{
name: "background-image",
value: "url(../../relative/image.png)",
priority: "",
offsets: [0, 47]
--- a/devtools/client/shared/test/unit/test_parseSingleValue.js
+++ b/devtools/client/shared/test/unit/test_parseSingleValue.js
@@ -17,17 +17,17 @@ const TEST_DATA = [
{input: "blue", expected: {value: "blue", priority: ""}},
{input: "blue !important", expected: {value: "blue", priority: "important"}},
{input: "blue!important", expected: {value: "blue", priority: "important"}},
{input: "blue ! important", expected: {value: "blue", priority: "important"}},
{
input: "blue ! important",
expected: {value: "blue", priority: "important"}
},
- {input: "blue !", expected: {value: "blue", priority: ""}},
+ {input: "blue !", expected: {value: "blue !", priority: ""}},
{input: "blue !mportant", expected: {value: "blue !mportant", priority: ""}},
{
input: " blue !important ",
expected: {value: "blue", priority: "important"}
},
{
input: "url(\"http://url.com/whyWouldYouDoThat!important.png\") !important",
expected: {
--- a/devtools/shared/css/parsing-utils.js
+++ b/devtools/shared/css/parsing-utils.js
@@ -297,17 +297,26 @@ function parseDeclarationsInternal(isCss
throw new Error("empty input string");
}
let lexer = getCSSLexer(inputString);
let declarations = [getEmptyDeclaration()];
let lastProp = declarations[0];
- let current = "", hasBang = false;
+ // This tracks the "!important" parsing state. The states are:
+ // 0 - haven't seen anything
+ // 1 - have seen "!", looking for "important" next (possibly after
+ // whitespace).
+ // 2 - have seen "!important"
+ let importantState = 0;
+ // This is true if we saw whitespace or comments between the "!" and
+ // the "important".
+ let importantWS = false;
+ let current = "";
while (true) {
let token = lexer.nextToken();
if (!token) {
break;
}
// Ignore HTML comment tokens (but parse anything they might
// happen to surround).
@@ -317,29 +326,33 @@ function parseDeclarationsInternal(isCss
// Update the start and end offsets of the declaration, but only
// when we see a significant token.
if (token.tokenType !== "whitespace" && token.tokenType !== "comment") {
if (lastProp.offsets[0] === undefined) {
lastProp.offsets[0] = token.startOffset;
}
lastProp.offsets[1] = token.endOffset;
- } else if (lastProp.name && !current && !hasBang &&
+ } else if (lastProp.name && !current && !importantState &&
!lastProp.priority && lastProp.colonOffsets[1]) {
// Whitespace appearing after the ":" is attributed to it.
lastProp.colonOffsets[1] = token.endOffset;
+ } else if (importantState === 1) {
+ importantWS = true;
}
if (token.tokenType === "symbol" && token.text === ":") {
+ // Either way, a "!important" we've seen is no longer valid now.
+ importantState = 0;
+ importantWS = false;
if (!lastProp.name) {
// Set the current declaration name if there's no name yet
lastProp.name = cssTrim(current);
lastProp.colonOffsets = [token.startOffset, token.endOffset];
current = "";
- hasBang = false;
// When parsing a comment body, if the left-hand-side is not a
// valid property name, then drop it and stop parsing.
if (inComment && !commentOverride &&
!isCssPropertyKnown(lastProp.name)) {
lastProp.name = null;
break;
}
@@ -352,68 +365,100 @@ function parseDeclarationsInternal(isCss
lastProp.terminator = "";
// When parsing a comment, if the name hasn't been set, then we
// have probably just seen an ordinary semicolon used in text,
// so drop this and stop parsing.
if (inComment && !lastProp.name) {
current = "";
break;
}
+ if (importantState === 2) {
+ lastProp.priority = "important";
+ } else if (importantState === 1) {
+ current += "!";
+ if (importantWS) {
+ current += " ";
+ }
+ }
lastProp.value = cssTrim(current);
current = "";
- hasBang = false;
+ importantState = 0;
+ importantWS = false;
declarations.push(getEmptyDeclaration());
lastProp = declarations[declarations.length - 1];
} else if (token.tokenType === "ident") {
- if (token.text === "important" && hasBang) {
- lastProp.priority = "important";
- hasBang = false;
+ if (token.text === "important" && importantState === 1) {
+ importantState = 2;
} else {
- if (hasBang) {
+ if (importantState > 0) {
current += "!";
+ if (importantWS) {
+ current += " ";
+ }
+ if (importantState === 2) {
+ current += "important ";
+ }
+ importantState = 0;
+ importantWS = false;
}
// Re-escape the token to avoid dequoting problems.
// See bug 1287620.
current += CSS.escape(token.text);
}
} else if (token.tokenType === "symbol" && token.text === "!") {
- hasBang = true;
+ importantState = 1;
} else if (token.tokenType === "whitespace") {
if (current !== "") {
- current += " ";
+ current = current.trimRight() + " ";
}
} else if (token.tokenType === "comment") {
if (parseComments && !lastProp.name && !lastProp.value) {
let commentText = inputString.substring(token.startOffset + 2,
token.endOffset - 2);
let newDecls = parseCommentDeclarations(isCssPropertyKnown, commentText,
token.startOffset,
token.endOffset);
// Insert the new declarations just before the final element.
let lastDecl = declarations.pop();
declarations = [...declarations, ...newDecls, lastDecl];
} else {
- current += " ";
+ current = current.trimRight() + " ";
}
} else {
+ if (importantState > 0) {
+ current += "!";
+ if (importantWS) {
+ current += " ";
+ }
+ if (importantState === 2) {
+ current += "important ";
+ }
+ importantState = 0;
+ importantWS = false;
+ }
current += inputString.substring(token.startOffset, token.endOffset);
}
}
// Handle whatever trailing properties or values might still be there
if (current) {
if (!lastProp.name) {
// Ignore this case in comments.
if (!inComment) {
// Trailing property found, e.g. p1:v1;p2:v2;p3
lastProp.name = cssTrim(current);
}
} else {
// Trailing value found, i.e. value without an ending ;
+ if (importantState === 2) {
+ lastProp.priority = "important";
+ } else if (importantState === 1) {
+ current += "!";
+ }
lastProp.value = cssTrim(current);
let terminator = lexer.performEOFFixup("", true);
lastProp.terminator = terminator + ";";
// If the input was unterminated, attribute the remainder to
// this property. This avoids some bad behavior when rewriting
// an unterminated comment.
if (terminator) {
lastProp.offsets[1] = inputString.length;