See bug 232868 comment 9
Created attachment 443671 [details] Patch
Created attachment 443672 [details] Patch
Comment on attachment 443672 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=443672&action=review > Source/WebCore/css/parser/CSSPropertyParser.cpp:-502 > // Every comma-separated list of identifiers is a valid will-change value, > // unless the list includes an explicitly disallowed identifier. > - while (true) { > - if (range.peek().type() != IdentToken) > + while (!range.atEnd()) { > + switch (range.peek().id()) { > + case CSSValueNone: > + case CSSValueAll: > + case CSSValueAuto: > return nullptr; > - CSSPropertyID propertyID = cssPropertyID(range.peek().value()); > - if (propertyID != CSSPropertyInvalid) { > - // Now "all" is used by both CSSValue and CSSPropertyValue. > - // Need to return nullptr when currentValue is CSSPropertyAll. > - if (propertyID == CSSPropertyWillChange || propertyID == CSSPropertyAll) > - return nullptr; > - values->append(CSSValuePool::singleton().createIdentifierValue(propertyID)); > - range.consumeIncludingWhitespace(); > - } else { It is not clear to me how this is better. The primary purpose of this loop is to consume property identifiers which the old code made clear. After your change the loop looks like it is about CSSValues and property IDs are a buried special case.
(In reply to Antti Koivisto from comment #3) > Comment on attachment 443672 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=443672&action=review > > > Source/WebCore/css/parser/CSSPropertyParser.cpp:-502 > > // Every comma-separated list of identifiers is a valid will-change value, > > // unless the list includes an explicitly disallowed identifier. > > - while (true) { > > - if (range.peek().type() != IdentToken) > > + while (!range.atEnd()) { > > + switch (range.peek().id()) { > > + case CSSValueNone: > > + case CSSValueAll: > > + case CSSValueAuto: > > return nullptr; > > - CSSPropertyID propertyID = cssPropertyID(range.peek().value()); > > - if (propertyID != CSSPropertyInvalid) { > > - // Now "all" is used by both CSSValue and CSSPropertyValue. > > - // Need to return nullptr when currentValue is CSSPropertyAll. > > - if (propertyID == CSSPropertyWillChange || propertyID == CSSPropertyAll) > > - return nullptr; > > - values->append(CSSValuePool::singleton().createIdentifierValue(propertyID)); > > - range.consumeIncludingWhitespace(); > > - } else { > > It is not clear to me how this is better. The primary purpose of this loop > is to consume property identifiers which the old code made clear. > > After your change the loop looks like it is about CSSValues and property IDs > are a buried special case. This is how the spec defines things, defines values first, and property IDs are just a special cases of <custom-ident>, so to me this is actually easier to read.
Comment on attachment 443672 [details] Patch Ok then!
Committed r285492 (244016@main): <https://commits.webkit.org/244016@main>
<rdar://problem/85198621>