Bug 248871 - [@property] Initial syntax parsing
Summary: [@property] Initial syntax parsing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antti Koivisto
URL:
Keywords: InRadar
: 192325 (view as bug list)
Depends on:
Blocks: 189692
  Show dependency treegraph
 
Reported: 2022-12-07 06:06 PST by Antti Koivisto
Modified: 2023-01-01 22:29 PST (History)
9 users (show)

See Also:


Attachments
Patch (84.15 KB, patch)
2022-12-07 08:38 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch for landing (84.22 KB, patch)
2022-12-07 09:40 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch for landing (84.21 KB, patch)
2022-12-07 09:43 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch for landing (84.21 KB, patch)
2022-12-07 09:48 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch for landing (84.21 KB, patch)
2022-12-07 09:51 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch for landing (84.22 KB, patch)
2022-12-07 09:55 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
Patch for landing (86.99 KB, patch)
2022-12-07 13:57 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2022-12-07 06:06:23 PST
Initial support for parsing syntax strings.
Comment 1 Antti Koivisto 2022-12-07 08:38:02 PST
Created attachment 463918 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2022-12-07 08:38:16 PST
<rdar://problem/103075062>
Comment 3 Darin Adler 2022-12-07 09:23:17 PST
Comment on attachment 463918 [details]
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.
Comment 4 Antti Koivisto 2022-12-07 09:40:05 PST
Created attachment 463920 [details]
Patch for landing
Comment 5 Antti Koivisto 2022-12-07 09:43:06 PST
Created attachment 463921 [details]
Patch for landing
Comment 6 Antti Koivisto 2022-12-07 09:46:14 PST
> No need for the braces here.

Some compilers fail when initialising the struct without providing the field value if it does not have braces. We talked about this in some earlier bug. But maybe those compilers are no longer relevant?
Comment 7 Antti Koivisto 2022-12-07 09:48:02 PST
Created attachment 463922 [details]
Patch for landing
Comment 8 Antti Koivisto 2022-12-07 09:50:52 PST
> 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.

Moved this inside the parsing callback so it can use proper isCSSSpace skipping.
Comment 9 Antti Koivisto 2022-12-07 09:51:13 PST
Created attachment 463923 [details]
Patch for landing
Comment 10 Antti Koivisto 2022-12-07 09:55:00 PST
Created attachment 463924 [details]
Patch for landing
Comment 11 Antti Koivisto 2022-12-07 13:57:45 PST
Created attachment 463928 [details]
Patch for landing
Comment 12 EWS 2022-12-07 17:21:19 PST
Committed 257525@main (be0b82ecc746): <https://commits.webkit.org/257525@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 463928 [details].
Comment 13 Tim Nguyen (:ntim) 2023-01-01 22:29:38 PST
*** Bug 192325 has been marked as a duplicate of this bug. ***