tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit 0e94c2e11a1729f52a6e381f7904df40be7be224
parent bd5c02afe931a43366951424a4ddb3dcd140b152
Author: Catherine Meade <cmeade@mozilla.com>
Date:   Thu,  2 Oct 2025 16:54:37 +0000

Bug 1987821 - Refactor `use-border-radius-tokens.mjs` to use shared helpers r=mstriemer

Differential Revision: https://phabricator.services.mozilla.com/D265544

Diffstat:
Mstylelint-rollouts.config.js | 5+++++
Mtools/lint/stylelint/stylelint-plugin-mozilla/helpers.mjs | 216+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------
Mtools/lint/stylelint/stylelint-plugin-mozilla/rules/use-border-color-tokens.mjs | 41++++++++++++++++++++++++++---------------
Mtools/lint/stylelint/stylelint-plugin-mozilla/rules/use-border-radius-tokens.mjs | 155++++++++++++++++++++++++++++++++++---------------------------------------------
Mtools/lint/stylelint/stylelint-plugin-mozilla/tests/use-border-radius-tokens.tests.mjs | 6++++++
5 files changed, 298 insertions(+), 125 deletions(-)

diff --git a/stylelint-rollouts.config.js b/stylelint-rollouts.config.js @@ -42,6 +42,7 @@ module.exports = [ "browser/themes/shared/notification-icons.css", "browser/themes/shared/search/searchbar.css", "browser/themes/shared/sidebar.css", + "browser/themes/shared/tabbrowser/content-area.css", "browser/themes/shared/tabbrowser/ctrlTab.css", "browser/themes/shared/tabbrowser/tabs.css", "browser/themes/shared/toolbarbuttons.css", @@ -67,6 +68,10 @@ module.exports = [ "toolkit/themes/shared/design-system/tokens-table.css", "toolkit/themes/shared/menu.css", "toolkit/themes/shared/popup.css", + "toolkit/components/certviewer/content/components/certificate-section.css", + "toolkit/content/widgets/infobar.css", + "toolkit/content/widgets/moz-box-common.css", + "toolkit/themes/shared/in-content/common-shared.css", ], }, { diff --git a/tools/lint/stylelint/stylelint-plugin-mozilla/helpers.mjs b/tools/lint/stylelint/stylelint-plugin-mozilla/helpers.mjs @@ -28,12 +28,8 @@ export function namespace(ruleName) { * The base list of allowed CSS values. */ export const ALLOW_LIST = [ - "0", - "auto", "inherit", "initial", - "none", - "normal", "revert", "revert-layer", "unset", @@ -47,7 +43,7 @@ export const ALLOW_LIST = [ */ export const createAllowList = (additionalAllows = []) => { - return [ALLOW_LIST, ...additionalAllows]; + return [...ALLOW_LIST, ...additionalAllows]; }; /** @@ -67,6 +63,25 @@ export const createTokenNamesArray = tokenCategoriesArray => }, []); /** + * Return raw values of tokens for the given categories. + * + * @param {string[]} tokenCategoriesArray + * @returns {object} + */ +export const createRawValuesObject = tokenCategoriesArray => + tokenCategoriesArray + .flatMap(category => tokensTable[category]) + .reduce((acc, token) => { + const val = String(token.value || "").trim(); + if (token.name && !val.startsWith("var(")) { + // some tokens refer to tokens in the table, + // let's move those out so our auto-fixes work + return { ...acc, [val]: `var(${token.name})` }; + } + return acc; + }, {}); + +/** * Collects local (in the same file) CSS properties from a * PostCSS object and returns those in object syntax. * @@ -86,14 +101,58 @@ export const getLocalCustomProperties = root => { }; /** + * Make breaks in CSS declaration to account for spaces. + * + * @param {string} value some CSS declaration + * @returns {string[]} + */ +const breakBySpace = value => { + const parsedValue = valueParser(String(value)); + + // parts becomes our return + const parts = []; + + // but we need a placeholder to work with + let currentPart = ""; + + // ValueParser sees space characters as nodes, so we can split without Regex + parsedValue.nodes.forEach(node => { + // this walks the node and when it finds a space, pushes the part + // to parts, then trims off that space because we don't want it + if (node.type === "space") { + if (currentPart.trim()) { + parts.push(currentPart.trim()); + currentPart = ""; + } + return; + } + + // but if no space, just add it + currentPart += valueParser.stringify(node); + }); + + // grab anything after the last space too + if (currentPart.trim()) { + parts.push(currentPart.trim()); + } + + return parts; +}; + +/** * Various checks for common design token and CSS content. * * @param {object} node object from PostCSS value-parser * @returns {boolean} */ +// checks if a node is a word export const isWord = node => node.type === "word"; + +// checks if a node is a function export const isFunction = node => node.type === "function"; + +// checks if a node is a `var()` function export const isVariableFunction = node => isFunction(node) && node.value === "var"; @@ -146,6 +205,11 @@ export const isAllowed = (value, allowList) => { return true; } + // Words inside var() should use tokens + if (valueParser(value).nodes.some(node => isVariableFunction(node))) { + return false; + } + // If the value is in the allowList but the string is CSS shorthand, e.g. `border` properties return valueParser(value).nodes.some( node => isWord(node) && allowList.includes(node.value) @@ -154,6 +218,7 @@ export const isAllowed = (value, allowList) => { /** * Checks if CSS value is a valid fallback expression, + * where we allow non-token fallbacks. * e.g., `var(--design-token, #000000);` * * @param {string} value some CSS declaration to match @@ -169,13 +234,22 @@ export const isValidFallback = (value, tokenCSS) => { return false; } - // isolate the token from the declaration - const firstWordIsToken = node.nodes.find(isWord); + // isolate the first word from the declaration and see if it is a token + const firstWord = node.nodes.find(isWord); + if (firstWord && isToken(`var(${firstWord.value})`, tokenCSS)) { + return true; + } - const firstWordIsTokenAsCSS = `var(${firstWordIsToken.value})`; + // isolate the fallback and see if it is a custom property + const fallbackProperty = node.nodes.find(isVariableFunction); + if (fallbackProperty) { + const fallbackWord = fallbackProperty.nodes.find(isWord); + if (fallbackWord && isToken(`var(${fallbackWord.value})`, tokenCSS)) { + return true; + } + } - // see if that token is in our token list - return tokenCSS.includes(firstWordIsTokenAsCSS); + return false; }); }; @@ -216,7 +290,7 @@ export const isValidLocalProperty = (value, cssCustomProperties, tokenCSS) => { export const trimValue = value => String(value).trim(); /** - * Checks if CSS value uses tokens correctly. + * Checks if CSS value uses tokens correctly (individually). * * @param {string} value some CSS declaration to match * @param {string[]} tokenCSS @@ -224,31 +298,131 @@ export const trimValue = value => String(value).trim(); * @param {string[]} allowList * @returns {boolean} */ -export const isValidTokenUsage = ( +export const isValidValue = ( value, tokenCSS, cssCustomProperties, - allowList = [] + allowList ) => { - const trimmedValue = trimValue(value); - + // assumes we've removed white space return ( - isToken(trimmedValue, tokenCSS) || - isAllowed(trimmedValue, allowList) || - isValidLocalProperty(trimmedValue, cssCustomProperties, tokenCSS) || - isValidFallback(trimmedValue, tokenCSS) || - tokenCSS.some(token => trimmedValue.includes(token)) + isToken(value, tokenCSS) || + isAllowed(value, allowList) || + isValidLocalProperty(value, cssCustomProperties, tokenCSS) || + isValidFallback(value, tokenCSS) ); }; /** + * Checks if CSS value uses tokens correctly (as a group). + * + * @param {string} value some CSS declaration to match + * @param {string[]} tokenCSS + * @param {object} cssCustomProperties + * @param {string[]} allowList defaults to the base list in this file + * @returns {boolean} + */ +export const isValidTokenUsage = ( + value, + tokenCSS, + cssCustomProperties, + allowList = ALLOW_LIST +) => { + const parsed = valueParser(value); + let isValid = false; + + parsed.walk(node => { + switch (node.type) { + case "word": { + // if the node is a word, check if it's an allowed word + isValid = isAllowed(node.value, allowList); + break; + } + case "function": { + // if the node is a function, check if it's a token + if (node.value == "var") { + let variableNode = `var(${node.nodes[0].value})`; + isValid = + isToken(variableNode, tokenCSS) || + isValidLocalProperty(variableNode, cssCustomProperties, tokenCSS); + } + break; + } + default: { + break; + } + } + return !isValid; + }); + + return isValid; +}; + +/** * Checks if CSS value uses color tokens correctly. * * @param {string} value some CSS declaration to match * @returns {boolean} */ -export const dontUseRawColors = value => { +export const usesRawColors = value => { const trimmedValue = trimValue(value); return containsHexColor(trimmedValue) || containsColorFunction(trimmedValue); }; + +/** + * Checks if CSS value is a valid fallback expression, + * where we allow disallow token fallbacks. + * e.g., `var(--design-token, --local-token);` + * + * @param {string} value some CSS declaration to match + * @param {string[]} rawValueToTokenValue an object of values to check + * @returns {boolean} + */ +export const usesRawFallbackValues = (value, rawValueToTokenValue) => { + if (value.includes("var(") && value.includes(",")) { + for (const token of Object.keys(rawValueToTokenValue)) { + if (value.includes(token)) { + return true; + } + } + } + return false; +}; + +/** + * Checks if all values in a shorthand declaration should be tokens. + * Stricter than isTokenPart, for logical CSS properties that allow + * mixed shorthand values. + * e.g., `border-radius: 1px 0 2px;` + * + * @param {string} value some CSS declaration to check + * @param {string[]} tokenCSS + * @param {object} cssCustomProperties + * @param {string[]} allowList + * @returns {boolean} + */ +export const usesRawShorthandValues = ( + value, + tokenCSS, + cssCustomProperties, + allowList = ALLOW_LIST +) => { + const parts = breakBySpace(String(value)); + + // only check shorthand, not single values + if (parts.length <= 1) { + return false; + } + + // look at each part and see if it is a valid value + // all parts must be valid + return !parts.every(part => { + return isValidValue( + trimValue(part), + tokenCSS, + cssCustomProperties, + allowList + ); + }); +}; diff --git a/tools/lint/stylelint/stylelint-plugin-mozilla/rules/use-border-color-tokens.mjs b/tools/lint/stylelint/stylelint-plugin-mozilla/rules/use-border-color-tokens.mjs @@ -8,7 +8,7 @@ import { namespace, createTokenNamesArray, isValidTokenUsage, - dontUseRawColors, + usesRawColors, createAllowList, getLocalCustomProperties, } from "../helpers.mjs"; @@ -29,39 +29,49 @@ const meta = { fixable: false, }; -// Gather an array of `['--token-name']` and the ready css `['var(--token-name)']` +// Gather an array of the ready css `['var(--token-name)']` const INCLUDE_CATEGORIES = ["border-color", "border", "outline"]; const tokenCSS = createTokenNamesArray(INCLUDE_CATEGORIES); // Allowed border-color values in CSS -const ALLOW_LIST = createAllowList(["transparent", "currentColor"]); - -const CSS_PROPERTIES = [ +const ALLOW_LIST = createAllowList([ + "transparent", + "currentColor", + "auto", + "normal", + "none", +]); + +const SHORTHAND_CSS_PROPERTIES = [ "border", - "border-color", - "outline", - "outline-color", "border-top", "border-right", "border-bottom", "border-left", + "border-block", + "border-block-start", + "border-block-end", + "border-inline", + "border-inline-start", + "border-inline-end", + "outline", +]; + +const CSS_PROPERTIES = [ + "border-color", + "outline-color", "border-top-color", "border-right-color", "border-bottom-color", "border-left-color", - "border-block", "border-block-color", - "border-block-start", "border-block-start-color", - "border-block-end", "border-block-end-color", - "border-inline", "border-inline-color", - "border-inline-start", "border-inline-start-color", - "border-inline-end", "border-inline-end-color", + ...SHORTHAND_CSS_PROPERTIES, ]; const ruleFunction = primaryOption => { @@ -70,6 +80,7 @@ const ruleFunction = primaryOption => { actual: primaryOption, possible: [true], }); + if (!validOptions) { return; } @@ -92,7 +103,7 @@ const ruleFunction = primaryOption => { cssCustomProperties, ALLOW_LIST ) && - !dontUseRawColors(declarations.value) + !usesRawColors(declarations.value) ) { return; } diff --git a/tools/lint/stylelint/stylelint-plugin-mozilla/rules/use-border-radius-tokens.mjs b/tools/lint/stylelint/stylelint-plugin-mozilla/rules/use-border-radius-tokens.mjs @@ -3,8 +3,16 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ import stylelint from "stylelint"; import valueParser from "postcss-value-parser"; -import { namespace } from "../helpers.mjs"; -import { tokensTable } from "../../../../../toolkit/themes/shared/design-system/tokens-table.mjs"; +import { + namespace, + createTokenNamesArray, + createRawValuesObject, + isValidTokenUsage, + getLocalCustomProperties, + usesRawFallbackValues, + usesRawShorthandValues, + createAllowList, +} from "../helpers.mjs"; const { utils: { report, ruleMessages, validateOptions }, @@ -22,79 +30,32 @@ const meta = { fixable: true, }; -const tableData = tokensTable["border-radius"]; - -const tokenMaps = tableData.reduce( - (acc, item) => { - const tokenVar = `var(${item.name})`; - acc.valueToTokenVariable[item.value] = tokenVar; - acc.tokenVariableToValue[tokenVar] = item.value; - return acc; - }, - { - valueToTokenVariable: { - "50%": "var(--border-radius-circle)", - "100%": "var(--border-radius-circle)", - "1000px": "var(--border-radius-circle)", - }, - tokenVariableToValue: {}, - } -); - -const { valueToTokenVariable, tokenVariableToValue } = tokenMaps; - -const ALLOW_LIST = ["0", "initial", "unset", "inherit"]; - -const isToken = val => !!tokenVariableToValue[val]; -const isWord = node => node.type === "word"; -const isVarFunction = node => node.type === "function" && node.value === "var"; - -const isValidLocalVariable = (val, localCssVars) => { - const parsed = valueParser(val); - let cssVar = null; - - parsed.walk(node => { - if (isVarFunction(node)) { - const args = node.nodes.filter(isWord); - if (args.length) { - cssVar = args[0].value; - } - } - }); - - if (cssVar && localCssVars[cssVar]) { - return isToken(localCssVars[cssVar]); - } - - return false; -}; - -const isValidFallback = val => { - if (val.includes("var(") && val.includes(",")) { - for (const token of Object.keys(tokenVariableToValue)) { - if (val.includes(token)) { - return true; - } - } - } - return false; -}; - -const isValidValue = (val, localCssVars) => - isToken(val) || - ALLOW_LIST.includes(val) || - isValidLocalVariable(val, localCssVars) || - isValidFallback(val); - -const isValidTokenUsage = (val, localCssVars) => { - if (isValidValue(val, localCssVars)) { - return true; - } - - const parts = val.split(/\s+/); - return ( - parts.length > 1 && parts.every(part => isValidValue(part, localCssVars)) - ); +// Gather an array of `['--token-name']` and the ready css `['var(--token-name)']` +const INCLUDE_CATEGORIES = ["border-radius"]; + +const tokenCSS = createTokenNamesArray(INCLUDE_CATEGORIES); + +// Allowed border-color values in CSS +const ALLOW_LIST = createAllowList(["0"]); + +const CSS_PROPERTIES = [ + "border-radius", + "border-top-left-radius", + "border-top-right-radius", + "border-bottom-right-radius", + "border-bottom-left-radius", + "border-start-start-radius", + "border-start-end-radius", + "border-end-start-radius", + "border-end-end-radius", +]; + +// some circular values aren't in our token tree, so we'll append them +const RAW_VALUE_TO_TOKEN_VALUE = { + ...createRawValuesObject(INCLUDE_CATEGORIES), + "50%": "var(--border-radius-circle)", + "100%": "var(--border-radius-circle)", + "1000px": "var(--border-radius-circle)", }; const ruleFunction = primaryOption => { @@ -108,32 +69,46 @@ const ruleFunction = primaryOption => { return; } - const localCssVars = {}; - // Walk declarations once to generate a lookup table of variables. - root.walkDecls(decl => { - if (decl.prop.startsWith("--")) { - localCssVars[decl.prop] = decl.value; - } - }); + const cssCustomProperties = getLocalCustomProperties(root); // Walk declarations again to detect non-token values. - root.walkDecls("border-radius", decl => { - if (isValidTokenUsage(decl.value, localCssVars)) { + root.walkDecls(declarations => { + // If the property is not in our list to check, skip it. + if (!CSS_PROPERTIES.includes(declarations.prop)) { + return; + } + + // Otherwise, see if we are using the tokens correctly + if ( + isValidTokenUsage( + declarations.value, + tokenCSS, + cssCustomProperties, + ALLOW_LIST + ) && + !usesRawFallbackValues(declarations.value, RAW_VALUE_TO_TOKEN_VALUE) && + !usesRawShorthandValues( + declarations.value, + tokenCSS, + cssCustomProperties, + ALLOW_LIST + ) + ) { return; } report({ - message: messages.rejected(decl.value), - node: decl, + message: messages.rejected(declarations.value), + node: declarations, result, ruleName, fix: () => { - const val = valueParser(decl.value); + const val = valueParser(declarations.value); let hasFixes = false; val.walk(node => { if (node.type == "word") { - const token = valueToTokenVariable[node.value]; + const token = RAW_VALUE_TO_TOKEN_VALUE[node.value.trim()]; if (token) { hasFixes = true; node.value = token; @@ -141,14 +116,16 @@ const ruleFunction = primaryOption => { } }); if (hasFixes) { - decl.value = val.toString(); + declarations.value = val.toString(); } }, }); }); }; }; + ruleFunction.ruleName = ruleName; ruleFunction.messages = messages; ruleFunction.meta = meta; + export default ruleFunction; diff --git a/tools/lint/stylelint/stylelint-plugin-mozilla/tests/use-border-radius-tokens.tests.mjs b/tools/lint/stylelint/stylelint-plugin-mozilla/tests/use-border-radius-tokens.tests.mjs @@ -165,6 +165,12 @@ testRule({ description: "1000px value should be fixed to use design token.", }, { + code: ".a { border-radius: 9999px; }", + fixed: ".a { border-radius: var(--border-radius-circle); }", + message: messages.rejected("9999px"), + description: "9999px value should be fixed to use design token.", + }, + { code: ".a { border-radius: 4px; }", fixed: ".a { border-radius: var(--border-radius-small); }", message: messages.rejected("4px"),