Bug 229915 – [css-masking] Add a temporarily prefixed property for mask-mode, aliased to -webkit-mask-source-type

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Simon Fraser (smfr)]

Add a temporarily prefixed property for mask-mode, aliased to -webkit-mask-source-type

[Simon Fraser (smfr)]

Created attachment 437344 [details]
Patch

[Simon Fraser (smfr)]

*** Bug 229899 has been marked as a duplicate of this bug. ***

[Antti Koivisto]

Comment on attachment 437344 [details]
Patch

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

> Source/WebCore/css/makeprop.pl:1307
> -  $setterContent .= $indent . "static void applyValue" . $nameToId{$name} . "(BuilderState& builderState, CSSValue& value)\n";
> +  $setterContent .= $indent . "static void applyValue" . $nameToId{$name} . "(CSSPropertyID property, BuilderState& builderState, CSSValue& value)\n";
>    $setterContent .= $indent . "{\n";
> +  
> +  if (!exists $propertiesWithStyleBuilderOptions{$name}{"fill-layer-property"}) {
> +    $setterContent .= $indent . "    UNUSED_PARAM(property);\n";
> +  }

If I understand correctly this adds the property argument to every one of the 647 generated applyValueFoo functions while it can be statically determined in all cases except one. This doesn't seem ideal.

The code generation could be check for 'synonym' concept and only generate the property parameter for it.

> Source/WebCore/css/makeprop.pl:1385
> +  next if (exists $synonyms{$name});

I suppose an alternative would be to just generate a separate function for the synonym and do the synonym name mapping when generating the function. This would avoid having to pass the propertyid dynamically and might be generally simpler.

[Simon Fraser (smfr)]

Created attachment 437364 [details]
Patch

[Simon Fraser (smfr)]

(In reply to Antti Koivisto from comment #3)

> > Source/WebCore/css/makeprop.pl:1385
> > +  next if (exists $synonyms{$name});
> 
> I suppose an alternative would be to just generate a separate function for
> the synonym and do the synonym name mapping when generating the function.
> This would avoid having to pass the propertyid dynamically and might be
> generally simpler.

I decided for now it was simplest just to pass the CSSPropertyID as an argument to all the fill layer value applier functions, since their implementations already had a hardcoded propertyID that can now just use the argument.

[Simon Fraser (smfr)]

Created attachment 437367 [details]
Patch

[Antti Koivisto]

Comment on attachment 437367 [details]
Patch

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

> Source/WebCore/css/makeprop.pl:306
>                      $nameToAliases{$name} = $codegenProperties->{"aliases"};
> +                } elsif ($codegenOptionName eq "synonym") {
> +                  $synonyms{$name} = 1;
> +                  push @{$namesToSynonyms{$codegenProperties->{"synonym"}}}, $name;
>                  } elsif ($styleBuilderOptions{$codegenOptionName}) {

weird indentation

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/282058/webkit

[Radar WebKit Bug Importer]

<rdar://problem/82795719>

[Aakash Jain]

(In reply to Simon Fraser (smfr) from comment #8)
> https://trac.webkit.org/changeset/282058/webkit
This seems to have broken two layout tests on iOS:
- imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext.html
- imported/w3c/web-platform-tests/css/cssom/getComputedStyle-detached-subtree.html

History: https://results.webkit.org/?suite=layout-tests&suite=layout-tests&test=imported%2Fw3c%2Fweb-platform-tests%2Fcss%2Fcssom%2Fcssstyledeclaration-csstext.html&test=imported%2Fw3c%2Fweb-platform-tests%2Fcss%2Fcssom%2FgetComputedStyle-detached-subtree.html

EWS indicated these failures on previous patch in https://ews-build.webkit.org/#/builders/51/builds/20605

Patch was landed manually and might have been different from the one which passed EWS.

[Aakash Jain]

Separately, please try to use commit-queue. We are trying to discourage manual commits. If you need to land a patch quickly, you can use fast-cq mode (See https://lists.webkit.org/pipermail/webkit-dev/2021-April/031782.html).

[WebKit Commit Bot]

Re-opened since this is blocked by bug 229978

[Simon Fraser (smfr)]

Created attachment 437447 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 437509 [details]
Patch

[EWS]

Committed r282143 (241440@main): <https://commits.webkit.org/241440@main>

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