Rename some style keyword constants to be consistent with style keywords

RESOLVED FIXED in Firefox 49

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: KiChjang, Assigned: KiChjang)

Tracking

({css-moz})

Trunk
mozilla49
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 fixed)

Details

Attachments

(3 attachments, 5 obsolete attachments)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160503215307




Expected results:

Style keyword constants like NS_STYLE_FLOAT_EDGE_CONTENT should be renamed as NS_STYLE_FLOAT_EDGE_CONTENT_BOX in order to be consistent with eCSSKeyword_content_box (necessary for stylo).
Keywords: css-moz
Version: 46 Branch → Trunk
Assignee: nobody → kungfukeith11
Status: UNCONFIRMED → NEW
Component: Untriaged → CSS Parsing and Computation
Ever confirmed: true
Product: Firefox → Core
Blocks: stylo
Comment on attachment 8753257 [details] [diff] [review]
0001-Rename-moz-float-edge-constants.patch

Review of attachment 8753257 [details] [diff] [review]:
-----------------------------------------------------------------

Patch looks good, thanks!

Can you attach a patch in hg format?  Since someone will need to check in the patch on your behalf, it's good to get it as close as possible to the final patch that the sheriffs will import.  I use the git-patch-to-hg-patch tool from https://github.com/mozilla/moz-git-tools to do this, when working from a git gecko-dev checkout, to take my top commit and turn it into an hg patch:

  $ git format-patch -U8 -pk --stdout HEAD~1 | git-patch-to-hg-patch > a.patch

(You'd think I'd have an alias for this but I don't.)  The commit line should be of the form "Bug NNNNNNN - Rename -moz-float [...] with keywords. r=heycam" though in general you don't need to fill in the bug number and r=foo (the sherrifs can fill those in).

Also, please try to find a reviewer for the patch when attaching it.  Unlike in the servo repository, a random reviewer will not be assigned automatically, so it's possible for patches to slip through the cracks without an explicit reviewer.  The bugzilla interface can suggest appropriate reviewers for the component the bug is filed in.
Attachment #8753257 - Flags: review+
When you have your updated patch attached, set the checkin-needed keyword in the bug and the sheriffs will land the patch for you.

I've pushed the patch to try (another thing which Servo's infrastructure will do automatically for you but here in Gecko-land needs to be done automatically, unless you use mozreview): https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1749fdfbd8e
Just a heads-up, this still requires a bit more renaming for -moz-appearance, and I'm working on it. Can my first patch be submitted to the main repo in the meantime though?
Yes, that's fine.  If you want to attach a few separate patches to this bug you can add the leave-open keyword and keep working in this bug.  Or a separate bug is fine, too.  (I tend to use separate bugs if I have part work that's landable, just to avoid confusion about the bug status.)
Status: NEW → ASSIGNED
Flags: needinfo?(cam)
Flags: needinfo?(cam)
Attachment #8753308 - Flags: review?(cam)
Attachment #8753308 - Flags: review?(bobbyholley)
Attachment #8753317 - Flags: review?(cam)
Attachment #8753317 - Flags: review?(bobbyholley)
Attachment #8753317 - Attachment is obsolete: true
Attachment #8753317 - Flags: review?(cam)
Attachment #8753317 - Flags: review?(bobbyholley)
Attachment #8753318 - Flags: review?(cam)
Attachment #8753318 - Flags: review?(bobbyholley)
Attachment #8753257 - Attachment is obsolete: true
Attachment #8753318 - Attachment is obsolete: true
Attachment #8753318 - Flags: review?(cam)
Attachment #8753318 - Flags: review?(bobbyholley)
Attachment #8753400 - Flags: review?(cam)
Attachment #8753400 - Flags: review?(bobbyholley)
Attachment #8753308 - Flags: review?(cam)
Attachment #8753308 - Flags: review?(bobbyholley)
Attachment #8753308 - Flags: review+
Comment on attachment 8753400 [details] [diff] [review]
Rename-moz-appearance-constants.patch

Review of attachment 8753400 [details] [diff] [review]:
-----------------------------------------------------------------

I kinda skimmed this with some spot-checking, but assuming you did this in some sort of automated way (i.e. find+replace) it should be fine. Thanks for doing this!
Attachment #8753400 - Flags: review?(cam)
Attachment #8753400 - Flags: review?(bobbyholley)
Attachment #8753400 - Flags: review+
Doing an all-platform try build for the platform-specific -moz-appearance stuff. I'll push to inbound when it's green.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9406eccae2f3
Attachment #8753400 - Attachment is obsolete: true
Attachment #8753412 - Flags: review?(cam)
Attachment #8753412 - Flags: review?(bobbyholley)
Attachment #8753412 - Flags: review?(cam)
Attachment #8753412 - Flags: review?(bobbyholley)
Attachment #8753412 - Flags: review+
Looks like this just collided with bug 1192053, which is pretty unlucky. Needs a rebase.
Flags: needinfo?(kungfukeith11)
Attachment #8753412 - Attachment is obsolete: true
Flags: needinfo?(kungfukeith11)
Attachment #8753759 - Flags: review?(bobbyholley)
Attachment #8753759 - Flags: review?(bobbyholley) → review+
https://hg.mozilla.org/mozilla-central/rev/e995b22f325e
https://hg.mozilla.org/mozilla-central/rev/9c7f405a2cb6
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Sorry, I seem to have missed 1 property and mistaken another 1. Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Superseded by 1274339.
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1274339
Attachment #8754473 - Flags: review?(bobbyholley)
Well, not quite a dupe because parts of this already landed.
Resolution: DUPLICATE → FIXED
You need to log in before you can comment on or make changes to this bug.