Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(style): Limit allowed attributes on <a> elements #4035

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/AudioContext.json
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@
"support": {
"chrome": {
"version_added": false,
"notes": "See <a href='https://crbug.com/432248' title='Issue 432248 - AudioContext needs to support optional sample rate parameter.'>issue 432248</a> for Chrome support."
"notes": "See <a href='https://crbug.com/432248'>issue 432248</a> for Chrome support."
},
"chrome_android": {
"version_added": null
Expand Down
2 changes: 1 addition & 1 deletion api/AudioContextOptions.json
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@
"support": {
"chrome": {
"version_added": false,
"notes": "See <a href='https://crbug.com/432248' title='Issue 432248 - AudioContext needs to support optional sample rate parameter.'>issue 432248</a> for Chrome support."
"notes": "See <a href='https://crbug.com/432248'>issue 432248</a> for Chrome support."
},
"chrome_android": {
"version_added": null
Expand Down
54 changes: 27 additions & 27 deletions api/BatteryManager.json

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions api/CharacterData.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
},
"ChildNode": {
"__compat": {
"description": "Implements <a href='https://developer.mozilla.org//docs/Web/API/ChildNode'><code>ChildNode</code></a> Interface",
"description": "Implements <a href='https://developer.mozilla.org/docs/Web/API/ChildNode'><code>ChildNode</code></a> Interface",
"support": {
"chrome": {
"version_added": true
Expand All @@ -65,7 +65,7 @@
},
"firefox": {
"version_added": "25",
"notes": "Two properties, <code>nextElementSibling</code> and <code>previousElementSibling</code>, have been moved to the <a href='https://developer.mozilla.org/docs/Web/API/NonDocumentTypeChildNode><code>NonDocumentTypeChildNode</code></a> interface, also implemented by <code>CharacterData</code>."
"notes": "Two properties, <code>nextElementSibling</code> and <code>previousElementSibling</code>, have been moved to the <a href='https://developer.mozilla.org/docs/Web/API/NonDocumentTypeChildNode'><code>NonDocumentTypeChildNode</code></a> interface, also implemented by <code>CharacterData</code>."
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This href attribute had an unclosed ', which broke the compatibility table: https://developer.mozilla.org/docs/Web/API/CharacterData#Browser_compatibility

},
"firefox_android": {
"version_added": "25",
Expand Down
4 changes: 2 additions & 2 deletions css/properties/transition.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"notes": [
"Before Firefox 57, transitions do not work when transitioning from a <a href='https://developer.mozilla.org/docs/Web/CSS/text-shadow'><code>text-shadow</code></a> with a color specified to a <code>text-shadow</code> without a color specified (see <A href='https://bugzil.la/726550'>bug 726550</a>).",
"Before Firefox 57, cancelling a filling animation (for example, with <code>animation-fill-mode: forwards</code> set) can trigger a transition set on the same element, although only once (see <a href='https://bugzil.la/1192592'>bug 1192592</a> and <a href='https://bug1192592.bmoattachments.org/attachment.cgi?id=8843824'>these test cases</a> for more information).",
"Before Firefox 57, the <a href='https://developer.mozilla.org/docs/Web/CSS/background-position'><code>background-position</code></a> property can't be transitioned between two values containing different numbers of <a href='https://developer.mozilla.org/docs/Web/CSS/position_value' t><code>&lt;position&gt;</code></a> values, for example <code>background-position: 10px 10px;</code> and <code>background-position: 20px 20px, 30px 30px;</code> (see <a href='https://bugzil.la/1390446'>bug 1390446</a>)."
"Before Firefox 57, the <a href='https://developer.mozilla.org/docs/Web/CSS/background-position'><code>background-position</code></a> property can't be transitioned between two values containing different numbers of <a href='https://developer.mozilla.org/docs/Web/CSS/position_value'><code>&lt;position&gt;</code></a> values, for example <code>background-position: 10px 10px;</code> and <code>background-position: 20px 20px, 30px 30px;</code> (see <a href='https://bugzil.la/1390446'>bug 1390446</a>)."
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These <a> elements had an extra empty t attribute for whatever reason.

]
},
{
Expand Down Expand Up @@ -76,7 +76,7 @@
"notes": [
"Before Firefox 57, transitions do not work when transitioning from a <a href='https://developer.mozilla.org/docs/Web/CSS/text-shadow'><code>text-shadow</code></a> with a color specified to a <code>text-shadow</code> without a color specified (see <A href='https://bugzil.la/726550'>bug 726550</a>).",
"Before Firefox 57, cancelling a filling animation (for example, with <code>animation-fill-mode: forwards</code> set) can trigger a transition set on the same element, although only once (see <a href='https://bugzil.la/1192592'>bug 1192592</a> and <a href='https://bug1192592.bmoattachments.org/attachment.cgi?id=8843824'>these test cases</a> for more information).",
"Before Firefox 57, the <a href='https://developer.mozilla.org/docs/Web/CSS/background-position'><code>background-position</code></a> property can't be transitioned between two values containing different numbers of <a href='https://developer.mozilla.org/docs/Web/CSS/position_value' t><code>&lt;position&gt;</code></a> values, for example <code>background-position: 10px 10px;</code> and <code>background-position: 20px 20px, 30px 30px;</code> (see <a href='https://bugzil.la/1390446'>bug 1390446</a>)."
"Before Firefox 57, the <a href='https://developer.mozilla.org/docs/Web/CSS/background-position'><code>background-position</code></a> property can't be transitioned between two values containing different numbers of <a href='https://developer.mozilla.org/docs/Web/CSS/position_value'><code>&lt;position&gt;</code></a> values, for example <code>background-position: 10px 10px;</code> and <code>background-position: 20px 20px, 30px 30px;</code> (see <a href='https://bugzil.la/1390446'>bug 1390446</a>)."
]
},
{
Expand Down
119 changes: 119 additions & 0 deletions test/test-style.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ function orderSupportBlock(key, value) {
return value;
}

/**
* @param {string} str
*/
function escapeInvisibles(str) {
const invisibles = [
['\b', '\\b'],
Expand All @@ -44,6 +47,48 @@ function escapeInvisibles(str) {
return finalString;
}

/**
* Gets the row and column matching the index in a string.
*
* @param {string} str
* @param {number} index
* @return {[number, number] | [null, null]}
*/
function indexToPos(str, index) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imported this helper function from #3913.

let line = 1, col = 1;
let prevChar = null;

if (
typeof str !== 'string' ||
typeof index !== 'number' ||
index > str.length
) {
return [null, null];
}

for (let i = 0; i < index; i++) {
let char = str[i];
switch (char) {
case '\n':
if (prevChar === '\r') break;
case '\r':
line++;
col = 1;
break;
case '\t':
// Use JSON `tab_size` value from `.editorconfig`
col += 2;
break;
default:
col++;
break;
}
prevChar = char;
}

return [line, col];
}

/**
* @param {string} actual
* @param {string} expected
Expand Down Expand Up @@ -91,6 +136,80 @@ function testStyle(filename) {
console.error('\x1b[31m Browser name sorting – Error on line ' + jsonDiff(actual, expectedSorting));
}

{
// TODO: Lint other elements.
const regexp = new RegExp(String.raw`<(a)(?: (.*?))?>(.*?)</a>`, 'g');
/** @type {RegExpExecArray | null} */ let match;
/** @type {RegExpExecArray | null} */ let attrMatch;
while (!!(match = regexp.exec(actual))) {
const attrRegexp = /([^\x00-\x20\x7F-\x9F"'>/=\uFDD0-\uFDEF\uFFFE\uFFFF\u{1FFFE}\u{1FFFF}\u{2FFFE}\u{2FFFF}\u{3FFFE}\u{3FFFF}\u{4FFFE}\u{4FFFF}\u{5FFFE}\u{5FFFF}\u{6FFFE}\u{6FFFF}\u{7FFFE}\u{7FFFF}\u{8FFFE}\u{8FFFF}\u{9FFFE}\u{9FFFF}\u{AFFFE}\u{AFFFF}\u{BFFFE}\u{BFFFF}\u{CFFFE}\u{CFFFF}\u{DFFFE}\u{DFFFF}\u{EFFFE}\u{EFFFF}\u{FFFFE}\u{FFFFF}\u{10FFFE}\u{10FFFF}]+)(?: *= *('[^']*'|\\"[^"]*\\"|[^\x09\x0A\x0C\x0D\x20"'=<>`]+))?/ug;
const [
anchorLinkActual,
elementName,
attributesActual,
text,
] = match;
/** @type {Array<{name:string,nameStart:number,nameEnd:number,value?:string,valueEnd?:number}>} */
const badAttributes = [];
/** @type {string | null} */
let pos = null;

if (elementName !== elementName.toLowerCase()) {
hasErrors = true;
console.error(
`\x1b[33m Style %s - Use lowercase element name <%s>.\x1b[0m`,
(pos = indexToPos(match.input, match.index).join(':')),
elementName.toLowerCase(),
)
}

while(!!(attrMatch = attrRegexp.exec(attributesActual))) {
const [
attrFull,
attrName,
attrValue,
] = attrMatch;
const attrStart = attrMatch.index;
/** @type {{name:string,nameStart:number,nameEnd:number,value?:string,valueEnd?:number}} */
let attrDescriptor = {
name: attrName,
nameStart: attrStart,
nameEnd: attrStart + attrName.length,
};
if (attrValue !== undefined) {
attrDescriptor.value = attrValue;
attrDescriptor.valueEnd = attrStart + attrFull.length;
}
if (attrName !== 'href') {
// Disallow non-href attributes on <a> links
badAttributes.push(attrDescriptor);
}
}

if (badAttributes.length > 0) {
hasErrors = true;
console.error(
`\x1b[33m Style %s - Valid attributes for element <%s> are: href\x1b[0m`,
(pos || (pos = indexToPos(match.input, match.index).join(':'))),
elementName.toLowerCase(),
)
console.error(
`\x1b[33m Style %s - Element <%s> has disallowed attributes: %s\x1b[0m`,
(pos || (pos = indexToPos(match.input, match.index).join(':'))),
elementName.toLowerCase(),
badAttributes.reduce((badAttrs, {name, value}) => {
let result = name;
if (typeof value === 'string') {
result += '=' + value;
}
badAttrs.push(result);
return badAttrs;
}, /** @type {string[]} */ ([])).join(', '),
);
}
}
}

const bugzillaMatch = actual.match(String.raw`https?://bugzilla\.mozilla\.org/show_bug\.cgi\?id=(\d+)`);
if (bugzillaMatch) {
// use https://bugzil.la/1000000 instead
Expand Down