Bug 213171

Summary: [WPT] infrastructure/assumptions/html-elements.html fails due to changes in style when all: initial is used
Product: WebKit Reporter: Sam Weinig <sam>
Component: CSSAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: esprehn+autocc, ews-watchlist, glenn, gsnedders, gyuyoung.kim, joepeck, koivisto, macpherson, menard, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Testcase
none
WIP
none
Patch
none
Patch none

Description Sam Weinig 2020-06-13 11:17:50 PDT
Created attachment 401845 [details]
Testcase

WPT: infrastructure/assumptions/html-elements.html fails due to changes in style when all: initial is used.

Test: https://wpt.fyi/results/infrastructure/assumptions/html-elements.html?run_id=586650004&run_id=579020002&run_id=594530003&run_id=583030005

Issue: For the simple tree:

<style>
span.b {
  all: initial;
}
</style>
<span class="a"><span>
<span class="b"><span>

The computed values of the two spans differ for two properties: "stroke-color" and "-webkit-mask-box-image-slice"

Attached reduction / reproduction which tests all the computed properties for both spans. (Unselect the "Pass" checkbox to just see the two failing cases).
Comment 1 Sam Weinig 2020-06-13 11:19:31 PDT
The two failing cases are:

- Different value for stroke-color expected "rgb(0, 0, 0)" but got "rgba(0, 0, 0, 0)".
- Different value for -webkit-mask-box-image-slice expected "0" but got "0 fill".
Comment 2 Sam Weinig 2020-06-13 11:23:43 PDT
For stroke-color, the first thing that pops out to me is that in CSSProperties.json, "stroke-color" is set to: 

    "initial": "currentColor"

but, RenderStyle::initialStrokeColor() is set to:

    static Color initialStrokeColor() { return Color::transparent; }
Comment 3 Antti Koivisto 2020-06-15 11:59:29 PDT
Yeah, seems like a bug. The spec indicates 'transparent' is the correct value so I suppose CSSProperties.json is wrong here.
Comment 4 Sam Weinig 2020-06-15 13:30:33 PDT
(In reply to Antti Koivisto from comment #3)
> Yeah, seems like a bug. The spec indicates 'transparent' is the correct
> value so I suppose CSSProperties.json is wrong here.

Ok. Cool, that's easy to fix. For -webkit-mask-box-image-slice, it was a bit harder to track down due to macros, but it looks like it gets the fill = false from ApplyPropertyBorderImageModifier::applyInitialValue() while NinePieceImage when initialized with Type::Mask (as it is in StyleRareNonInheritedData) gets initialized with fill = true.

My guess is to match the one in NinePieceImage.
Comment 5 Sam Weinig 2020-06-15 14:31:31 PDT Comment hidden (obsolete)
Comment 6 Sam Weinig 2020-06-15 18:47:40 PDT
Created attachment 401966 [details]
Patch
Comment 7 Antti Koivisto 2020-06-15 22:43:34 PDT
Comment on attachment 401966 [details]
Patch

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

> Source/WebCore/css/CSSProperties.json:3734
> +                "initial": "initialStrokeColor",

I think calling RenderStyle::initialFoo is the default so just not specifying anything here should work too.

> Source/WebCore/style/StyleBuilderCustom.h:543
> -            image.setFill(false);
> +            image.setFill(type == BorderImage ? false : true);

type != BorderImage
Comment 8 Sam Weinig 2020-06-17 08:48:44 PDT
Created attachment 402110 [details]
Patch
Comment 9 EWS 2020-06-17 09:42:46 PDT
Committed r263156: <https://trac.webkit.org/changeset/263156>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 402110 [details].
Comment 10 Radar WebKit Bug Importer 2020-06-17 09:43:18 PDT
<rdar://problem/64451750>
Comment 11 Sam Weinig 2020-07-21 14:42:39 PDT
*** Bug 187052 has been marked as a duplicate of this bug. ***