1370797 - stylo: context-fill / context-stroke are not working when using SVG as an image on stylo

Status: RESOLVED FIXED

(Core :: SVG, defect, P1)

Description

[Tommy Kuo (away forever...)]

I try to open the following test cases with enabling `svg.context-properties.content.enabled`.

- layout/reftests/svg/as-image/context-fill-01.html
- layout/reftests/svg/as-image/context-stroke-01.html

The result color w/ stylo is blue (the fallback color), and the color w/o stylo is lime.

Comment 1

[Tommy Kuo (away forever...)]

I found we get the correct color (lime) in [1]. So, I think the problem doesn't happen when cascading.

[1]: https://searchfox.org/mozilla-central/source/servo/components/style/properties/gecko.mako.rs#441-446

Comment 2

[Tommy Kuo (away forever...)]

Attachment: context-fill-01.svg

I open this SVG directly, and `context-fill` works. I think this bug happens only using SVG as an image.

Comment 3

[Ting-Yu Lin [:TYLin] (UTC-7)]

`-moz-context-propreties` was marked as "internal=True" at [1], which doesn't get parsed in normal document. You should see 

INFO:style::error_reporting: Url:	(stylo: not supported)
19:3 Unsupported property declaration: '-moz-context-properties: fill;', found unexpected identifier fill

when running `RUST_LOG=style::error_reporting ./mach run file:///Users/tlin/Projects/gecko-dev/layout/reftests/svg/as-image/context-fill-01.html`

heycam, does stylo support property parsing by fliping gecko pref? Gecko flips "svg.context-properties.content.enabled" to true to parse "-moz-context-propreties" in reftests [2].

[1] http://searchfox.org/mozilla-central/rev/c49a70b53f67dd5550eec8a08793805f2aca8d42/servo/components/style/properties/longhand/inherited_svg.mako.rs#271
[2] http://searchfox.org/mozilla-central/rev/c49a70b53f67dd5550eec8a08793805f2aca8d42/layout/reftests/svg/as-image/reftest.list#60

Comment 4

[Cameron McCormack (:heycam)]

I think stylo does respond to those prefs being flipped.  It looks like the implementation of -moz-context-properties in inherited_svg.mako.rs doesn't understand the "fill" keyword, though...

Comment 5

[Tommy Kuo (away forever...)]

The value list pass into `CustomIdent::from_ident` is an exclusive list[1], so it doesn't mean we don't support "fill", "stroke", "fill-opacity" and "stroke-opacity". The value what we support is defined in [2], and we can see stylo already support "fill" and "stroke".

We still need to figure out why "context-fill" and "context-stroke" not working when SVG as an image.

[1]: http://searchfox.org/mozilla-central/rev/7cc377ce3f0a569c5c9b362d589705cd4fecc0ac/servo/components/style/properties/longhand/inherited_svg.mako.rs#290
[2]: http://searchfox.org/mozilla-central/rev/7cc377ce3f0a569c5c9b362d589705cd4fecc0ac/servo/components/style/properties/gecko.mako.rs#4110-4114

Comment 6

[Tommy Kuo (away forever...)]

I think the bug is caused by [1]. It made author can not use `-moz-context-properties`. I tent to remove this line to fix it.

[1]: http://searchfox.org/mozilla-central/rev/c49a70b53f67dd5550eec8a08793805f2aca8d42/servo/components/style/properties/longhand/inherited_svg.mako.rs#271

Comment 7

[Tommy Kuo (away forever...)]

Attachment: Bug 1370797 - Expose -moz-context-properties on stylo

Review commit: https://reviewboard.mozilla.org/r/150432/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/150432/

Comment 8

[Tommy Kuo (away forever...)]

Attachment: Bug 1370797 - Enable reftests of context-fill and context-stroke

Review commit: https://reviewboard.mozilla.org/r/150434/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/150434/

Comment 9

[Tommy Kuo (away forever...)]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1b2541652f4574adc214fd8347f7d9cb1e54ae8

Comment 10

[Cameron McCormack (:heycam)]

Comment on attachment 8879092 [details]
Bug 1370797 - Expose -moz-context-properties on stylo

https://reviewboard.mozilla.org/r/150432/#review155442

Comment 11

[Cameron McCormack (:heycam)]

Comment on attachment 8879093 [details]
Bug 1370797 - Enable reftests of context-fill and context-stroke

https://reviewboard.mozilla.org/r/150434/#review155444

Comment 12

[Tommy Kuo (away forever...)]

Comment on attachment 8879093 [details]
Bug 1370797 - Enable reftests of context-fill and context-stroke

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150434/diff/1-2/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49f8263e9220
Enable reftests of context-fill and context-stroke r=heycam

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/49f8263e9220