1617644 - Convert color-interpolation #defines to an enum class.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, task, P3)

Comment 1

[Philipp Zech]

So I replaced the original color-interpolation constants with a respective enum type. Starting from nStyleConsts.h, I tracked all necessary changes down to the other files. I also added the new type to ServoBindings.toml under rusty-enums and also added the necessary gecko_enum_prefix to inherited_svg.mako.rs . Now, structs.rs seems to be regenerated properly by using the new enum type, however, gecko.properties.rs still references the old constant, resulting in a failing build. My question now - what is necessary for that gecko.properties.rs will be regenerated and make use of the new enum type? I already went through previous submissions, however, could not find any further hint on what to do.

I also already tried building with calling mach clobber before mach build, but that also does not help.

Any hint would be greatly appreciated!

Comment 2

[Emilio Cobos Álvarez (:emilio)]

I think you're missing gecko_enum_prefix on the properties here: https://searchfox.org/mozilla-central/rev/b2ccce862ef38d0d150fcac2b597f7f20091a0c7/servo/components/style/properties/longhands/inherited_svg.mako.rs#37

Comment 3

[Emilio Cobos Álvarez (:emilio)]

(And below, on the color-interpolation-filters one).

But feel free to submit a WIP patch and I'm happy to take a look if that's not it :)

Comment 4

[Philipp Zech]

Oh snap! I actually went over that part, but apparently did not realize that I also have to add the prefix there; however, quite ironically, I managed to add it to color-interpolation-filters. Thanks a lot for that! The build now works, testing is under way and a patch will be submitted after that (given that the tests pass).

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Awesome! Btw, for future reference, when you're blocked on something it's better to use the "Request information from" checkbox in Bugzilla.

That adds a persistent notification so it's less easy to miss than an email :)

Comment 6

[Philipp Zech]

I see, thanks for the tip!

Comment 7

[Philipp Zech]

I applied my changes, the code builds, but running tests (mach test /layout) results in a bunch of errors when running tests from layout/base - could they be related to my changes (currently trying to figure out possible dependencies in the code, but it's a pretty huge repo...). When running tests as mach test layout/style the only test that fails is layout/style/test/test_visited_reftests.html - but I see no relation to my changes...anyways, if there are, clearly I'll investigate it.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

I recommend to upload the patch and I can send it through automation for you. You shouldn't run all the tests yourself :)

Note that some of the failures reported by the visited reftests are expected "TEST-EXPECTED-FAIL" or such. Because of the timing dependency on :visited (visited queries are async). Probably you get the same output without your patch, but we can always double-check :)

Comment 9

[Philipp Zech]

Done.

Comment 10

[Philipp Zech]

Attachment: Bug 1617644 - Convert color-interpolation #defines to an enum class. r=emilio

Comment 11

[Philipp Zech]

Attachment: Bug 1617644 - Convert color-interpolation #defines to an enum class. r=emilio

Apply emilio's comment.

Depends on D64813

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ea2a6d8b99e6
Convert color-interpolation #defines to an enum class. r=emilio

Comment 13

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/ea2a6d8b99e6