1354000 - stylo: stylo doesn't understand moz-prefixed -moz-crisp-edges

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P1)

Description

[Shing Lyu [:shinglyu]]

Stylo accepts "image-rendering:crisp-edges", not "image-rendering:-moz-crisp-edges". But gecko (and many testcases) still use -moz-crisp-edges. 

Should we:
1. Add both to the reftests?
2. Ship "crisp-edges" in Firefox?
3. Make Servo/Stylo understand "-moz-crisp-edges"?

Comment 1

[Shing Lyu [:shinglyu]]

Attachment: Affected test cases

Comment 2

[Bobby Holley (:bholley)]

(In reply to Shing Lyu [:shinglyu] from comment #0)
> Stylo accepts "image-rendering:crisp-edges", not
> "image-rendering:-moz-crisp-edges". But gecko (and many testcases) still use
> -moz-crisp-edges. 

Nice find! That looks like a lot of tests. Looks like Tommy is going to work on it?

> 
> Should we:
> 1. Add both to the reftests?
> 2. Ship "crisp-edges" in Firefox?
> 3. Make Servo/Stylo understand "-moz-crisp-edges"?

In general, we should do option (3). Changing the prefixes that Firefox ships is out of scope for stylo. I think this is probably a one-line annotation in the relevant .mako.rs file to enable the prefix on gecko builds.

Comment 3

[Chih-Hsuan Kuo [:tommykuo]]

Attachment: Bug 1354000 - (Part 1) Make stylo recognize -moz-crisp-edges keyword.

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

Comment 4

[Chih-Hsuan Kuo [:tommykuo]]

Attachment: Bug 1354000 - Update test result.

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

Comment 5

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8855664 [details]
Bug 1354000 - (Part 1) Make stylo recognize -moz-crisp-edges keyword.

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

Comment 6

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8855665 [details]
Bug 1354000 - Update test result.

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

Comment 7

[Cameron McCormack (:heycam)]

Comment on attachment 8855664 [details]
Bug 1354000 - (Part 1) Make stylo recognize -moz-crisp-edges keyword.

https://reviewboard.mozilla.org/r/127520/#review130260

Comment 8

[Cameron McCormack (:heycam)]

Comment on attachment 8855665 [details]
Bug 1354000 - Update test result.

https://reviewboard.mozilla.org/r/127522/#review130262

Nice!

Comment 9

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8855664 [details]
Bug 1354000 - (Part 1) Make stylo recognize -moz-crisp-edges keyword.

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

Comment 10

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8855665 [details]
Bug 1354000 - Update test result.

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

Comment 11

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8855664 [details]
Bug 1354000 - (Part 1) Make stylo recognize -moz-crisp-edges keyword.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127520/diff/3-4/

Comment 12

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8855665 [details]
Bug 1354000 - Update test result.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127522/diff/3-4/

Comment 13

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8855664 [details]
Bug 1354000 - (Part 1) Make stylo recognize -moz-crisp-edges keyword.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127520/diff/4-5/

Comment 14

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8855665 [details]
Bug 1354000 - Update test result.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127522/diff/4-5/

Comment 15

[Chih-Hsuan Kuo [:tommykuo]]

Hi Cameron, can you take a look the newer patch? I made some changes on it.

Comment 16

[Cameron McCormack (:heycam)]

Comment on attachment 8855664 [details]
Bug 1354000 - (Part 1) Make stylo recognize -moz-crisp-edges keyword.

https://reviewboard.mozilla.org/r/127520/#review131368

::: servo/components/style/properties/longhand/inherited_box.mako.rs:83
(Diff revisions 2 - 5)
> -                         "auto crisp-edges",
> +                         "auto",
>                           extra_gecko_values="optimizespeed optimizequality -moz-crisp-edges",
> -                         extra_servo_values="pixelated",
> +                         extra_servo_values="pixelated crisp-edges",
>                           custom_consts=image_rendering_custom_consts,
> -                         animatable=False,
> +                         animation_type="none",
> +                         needs_conversion="True",

Did you find this one was needed?  If not, please remove it.  The comment at http://searchfox.org/mozilla-central/rev/624d25b2980cf5f83452b040e6d664460f9b7ec5/servo/components/style/properties/helpers.mako.rs#401 (this from_gecko_keyword function is generated when needs_conversion="True") seems like it's only intended for properties set by presentation attributes.

Otherwise, the patch looks good.

Comment 17

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8855664 [details]
Bug 1354000 - (Part 1) Make stylo recognize -moz-crisp-edges keyword.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127520/diff/5-6/

Comment 18

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8855665 [details]
Bug 1354000 - Update test result.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127522/diff/5-6/

Comment 19

[Chih-Hsuan Kuo [:tommykuo]]

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

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c3e3315e68fb
(Part 1) Make stylo recognize -moz-crisp-edges keyword. r=heycam
https://hg.mozilla.org/integration/autoland/rev/979d89d569f2
(Part 2) Update test result. r=heycam

Comment 21

[Iris Hsiao [:ihsiao]]

backed out https://hg.mozilla.org/integration/autoland/rev/979d89d569f2 for touching things under servo/

https://hg.mozilla.org/integration/autoland/rev/d0e97ee3f273

Comment 22

[Chih-Hsuan Kuo [:tommykuo]]

Oh, I forgot to split them... Thx!

Comment 23

[Iris Hsiao [:ihsiao]]

backed out https://hg.mozilla.org/integration/autoland/rev/979d89d569f20b7530885e1f2be88dafd6cce9cc for stylo tests bustage

https://hg.mozilla.org/integration/autoland/rev/4e822b8d3409

Comment 24

[Chih-Hsuan Kuo [:tommykuo]]

Comment on attachment 8855665 [details]
Bug 1354000 - Update test result.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/127522/diff/6-7/

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c23ea4277b6
Update test result. r=heycam

Comment 26

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

https://hg.mozilla.org/mozilla-central/rev/4c23ea4277b6