Closed Bug 1311870 Opened 8 years ago Closed 7 years ago

Change property id of moz-prefixed properties to match their name

Categories

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

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox53 --- fixed

People

(Reporter: xidorn, Assigned: xidorn)

References

Details

Attachments

(3 files)

Unprefix column-rule properties in servo side.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Bug 1311870 - Change property id of moz-prefixed properties to match their name.
59 bytes, text/x-review-board-request
heycam
: review+
Details
Remove special case of converting from nsCSSPropertyID to PropertyId.
59 bytes, text/x-review-board-request
heycam
: review+
Details 
There are some moz-prefixed properties whose name does not have prefix "_moz_". We should probably fix that to make it easier for scripts to generate corresponding names (especially in the Servo side).
Taking it as I have a WIP patch locally.
Assignee: nobody → xidorn+moz
Priority: -- → P3
This also fixes several test failures in style system tests, because of some inconsistency.
I would much rather go in the other direction -- I don't want the _moz_ in the IDs.  It means there's less to change when we unprefix things.  (I think there's probably an open bug on that somewhere...)
(There's also bug 849657 on changing those IDs to use CamelCase so that we don't have to have a separate field in nsCSSPropList.h for them -- i.e., just using the third field for them instead of the second.  Then again, I guess this all changes with stylo.)
(In reply to David Baron :dbaron: ⌚️UTC-8 from comment #6)
> I would much rather go in the other direction -- I don't want the _moz_ in
> the IDs.  It means there's less to change when we unprefix things.  (I think
> there's probably an open bug on that somewhere...)

(And I've insisted on this in reviews of new properties -- I've just never gone back to fix all the old ones.)
On the other hand, maybe now that we're not using prefixing for "in development" properties, we shouldn't worry about making unprefixing easy.
... so I guess I'm ok with this change, although it was the opposite of previous plans.
Most of the things changed here are something we probably never want to unprefix. Also, for majority of the properties, there are just two places (nsCSSPropList and nsComputedDOMStylePropertyList) we need to change when we unprefix them, which doesn't seem to be a big deal, especially given that, as you said, we no longer do prefixing for "in development" properties.

On the other hand, having inconsistency in the name means we need to do some special cases in various places, which can be error-prone, and make it hard to catch changes.

You can see from the first patch, column-rule-* has already been unprefixed for a while in Gecko, but Servo code is still using the prefixed name which should already be caught if we don't have the special rule to strip the _moz_ prefix.

That special rule also leads to some inconsistency in result from CSSOM.
Comment on attachment 8820558 [details]
Unprefix column-rule properties in servo side.

https://reviewboard.mozilla.org/r/100046/#review100526
Attachment #8820558 - Flags: review?(cam) → review+
Comment on attachment 8820559 [details]
Bug 1311870 - Change property id of moz-prefixed properties to match their name.

https://reviewboard.mozilla.org/r/100048/#review100524

::: layout/style/nsComputedDOMStylePropertyList.h:276
(Diff revision 1)
>  COMPUTED_STYLE_PROP(border_bottom_colors,          BorderBottomColors)
>  COMPUTED_STYLE_PROP(border_left_colors,            BorderLeftColors)
>  COMPUTED_STYLE_PROP(border_right_colors,           BorderRightColors)
>  COMPUTED_STYLE_PROP(border_top_colors,             BorderTopColors)

Why don't we have "_moz" in the nsCSSPropertyID name for all of the properties mentioned in this section?
Comment on attachment 8820560 [details]
Remove special case of converting from nsCSSPropertyID to PropertyId.

https://reviewboard.mozilla.org/r/100050/#review100528
Attachment #8820560 - Flags: review?(cam) → review+
Comment on attachment 8820559 [details]
Bug 1311870 - Change property id of moz-prefixed properties to match their name.

https://reviewboard.mozilla.org/r/100048/#review100524

> Why don't we have "_moz" in the nsCSSPropertyID name for all of the properties mentioned in this section?

The patch was written months ago... I guess I just went through all properties that Servo side already added to avoid adding any special case for the enum type conversion.
Comment on attachment 8820559 [details]
Bug 1311870 - Change property id of moz-prefixed properties to match their name.

https://reviewboard.mozilla.org/r/100048/#review100540

Looks good, thanks!
Attachment #8820559 - Flags: review?(cam) → review+
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e218a5e2e015
Change property id of moz-prefixed properties to match their name. r=heycam
https://hg.mozilla.org/mozilla-central/rev/e218a5e2e015
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: