Bug 232868 – Use isValidCustomIdentifier in consumeWillChange

• Component: CSS
• Status: RESOLVED FIXED

[Tim Nguyen (:ntim)]

This is what the spec says: https://drafts.csswg.org/css-will-change/#will-change

> The <custom-ident> production used here excludes the keywords will-change, none, all, auto, scroll-position, and contents, in addition to the keywords normally excluded from <custom-ident>.

[Tim Nguyen (:ntim)]

Created attachment 443655 [details]
Patch

[Antti Koivisto]

Comment on attachment 443655 [details]
Patch

Can you either include the relevant WPT merge with the patch or merge the WPT first? Landing behaviour changes without tests is not nice.

[Tim Nguyen (:ntim)]

Created attachment 443657 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Tim Nguyen (:ntim)]

Created attachment 443659 [details]
Patch

[EWS]

Committed r285487 (244011@main): <https://commits.webkit.org/244011@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 443659 [details].

[Radar WebKit Bug Importer]

<rdar://problem/85193689>

[Antti Koivisto]

Comment on attachment 443655 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=443655&action=review

> Source/WebCore/css/parser/CSSPropertyParser.cpp:517
> +                if (!isValidCustomIdentifier(id))
> +                    return nullptr;
>                  // Append properties we don't recognize, but that are legal, as strings.
>                  values->append(consumeCustomIdent(range).releaseNonNull());

consumeCustomIdent should ASSERT(isValidCustomIdentifier) to catch all these.

[Antti Koivisto]

Comment on attachment 443659 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=443659&action=review

> Source/WebCore/css/parser/CSSPropertyParser.cpp:517
> +                if (!isValidCustomIdentifier(id))
> +                    return nullptr;
>                  // Append properties we don't recognize, but that are legal, as strings.
>                  values->append(consumeCustomIdent(range).releaseNonNull());

Actually since consumeCustomIdent already check isValidCustomIdentifier() a better fix here would be to just null chack consumeCustomIdent result