[webkit-reviews] review granted: [Bug 248871] [@property] Initial syntax parsing : [Attachment 463918] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 248871: [@property] Initial syntax parsing
https://bugs.webkit.org/show_bug.cgi?id=248871

Attachment 463918: Patch

https://bugs.webkit.org/attachment.cgi?id=463918&action=review




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 463918
  --> https://bugs.webkit.org/attachment.cgi?id=463918
Patch

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

> Source/WebCore/css/parser/CSSPropertySyntax.cpp:86
> +    auto string = syntax.stripWhiteSpace();

This should be using a function that strips the appropriate definition of space
for CSS, not String::stripWhiteSpace, which strips all Unicode whitespace. I’m
guessing the stripLeadingAndTrailingHTMLSpaces function is the right one to
use. Once nice thing about that is that it returns a StringView and so it’s
more efficient.

> Source/WebCore/css/parser/CSSPropertySyntax.h:51
> +	   AtomString ident { };

No need for the braces here.

> Source/WebCore/css/parser/CSSPropertySyntax.h:56
> +    static Definition parse(const String&);

I suggest this take a StringView, unless there’s some reason it can’t.