Closed Bug 1386326 Opened 7 years ago Closed 7 years ago

Remove unneeded constants for Aural CSS from nsStyleConsts.h

Categories

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

Product:
Core
Component:
CSS Parsing and Computation
Version:
Other Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox-esr52 --- wontfix
firefox55 --- wontfix
firefox56 --- wontfix
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: akriti.v10, Assigned: akriti.v10, Mentored)

References

Details

Attachments

(2 files, 5 obsolete files)

Bug 1386326 - Removed unwanted constants from nsStyleconsts.h
(deleted), text/x-review-board-request
manishearth
: review+
Details
rb169734.patch
(deleted), patch
manishearth
: review-
Details | Diff | Splinter Review
Attached file nsStyleConsts.h (obsolete) (deleted) —
Have Created a new enum class 'Azimuth' for a set of #define constants. Have made changes to nsStyleConsts.h , structs_debug.rs ,structs_release.rs
I'm not sure why those constants are still there. Everything from NS_STYLE_AZIMUTH_* through NS_STYLE_VOLUME_* should probably be removed instead. (It should have been removed in bug 649119.) Also, you need to attach patches rather than complete files.
Assignee: nobody → akriti.v10
Blocks: 1277133
Summary: Change #define constants to enum classes → Change #define constants to enum classes for Azimuth constants
Attached file structs_release.rs (obsolete) (deleted) —
Attached file structs_debug.rs (obsolete) (deleted) —
Also, there's no need to attach structs_debug.rs and structs_release.rs. Those are autogenerated and can get updated when doing a Servo PR. Please see: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch Thanks again!
Attached patch nsStyleConsts.h (obsolete) (deleted) — — Splinter Review
Attachment #8892506 - Attachment is obsolete: true
Attachment #8892509 - Attachment is obsolete: true
Attachment #8892511 - Attachment is obsolete: true
Flags: needinfo?(manishearth)
Attachment #8892815 - Flags: review?(manishearth)
Attachment #8892815 - Flags: review?(dbaron)
Comment on attachment 8892815 [details] [diff] [review] nsStyleConsts.h Please see the above link about how to produce a patch.
Attachment #8892815 - Flags: review?(manishearth)
Attachment #8892815 - Flags: review?(dbaron)
Attachment #8892815 - Flags: review-
Flags: needinfo?(manishearth)
Comment on attachment 8895937 [details] Bug 1386326 - Removed unwanted constants from nsStyleconsts.h https://reviewboard.mozilla.org/r/167210/#review172434 This doesn't fix the code where these are used, please ensure this compiles.
Attachment #8895937 - Flags: review-
Attached patch Constants changed to enum classes (obsolete) (deleted) — — Splinter Review
(In reply to Manish Goregaokar [:manishearth] from comment #8) > This doesn't fix the code where these are used, please ensure this compiles. As I said in comment 1, they're not used, so they should really be removed instead of converted.
Attached patch rb169734.patch (deleted) — — Splinter Review
An enum class "StyleFont" has been created instead of the following constants { #define NS_STYLE_FONT_SIZE_XXSMALL 0 #define NS_STYLE_FONT_SIZE_XSMALL 1 #define NS_STYLE_FONT_SIZE_SMALL 2 #define NS_STYLE_FONT_SIZE_MEDIUM 3 #define NS_STYLE_FONT_SIZE_LARGE 4 #define NS_STYLE_FONT_SIZE_XLARGE 5 #define NS_STYLE_FONT_SIZE_XXLARGE 6 #define NS_STYLE_FONT_SIZE_XXXLARGE 7 #define NS_STYLE_FONT_SIZE_LARGER 8 #define NS_STYLE_FONT_SIZE_SMALLER 9 } accordingly changes have been made in the connected files. The code compiles successfully
Attachment #8892815 - Attachment is obsolete: true
Attachment #8895937 - Attachment is obsolete: true
Attachment #8896757 - Attachment is obsolete: true
Flags: needinfo?(manishearth)
Summary: Change #define constants to enum classes for Azimuth constants → Change NS_STYLE_FONT_SIZE to StyleFont Enum class
Attachment #8898652 - Flags: review?(manishearth)
Comment on attachment 8898652 [details] [diff] [review] rb169734.patch Review of attachment 8898652 [details] [diff] [review]: ----------------------------------------------------------------- The integer value of this enum is actually relevant, let's not touch this one for now. Instead, please provide the previous patch with those constants removed.
Attachment #8898652 - Flags: review?(manishearth) → review-
Flags: needinfo?(manishearth)
Comment on attachment 8895937 [details] Bug 1386326 - Removed unwanted constants from nsStyleconsts.h https://reviewboard.mozilla.org/r/167210/#review177448
Attachment #8895937 - Flags: review+
Summary: Change NS_STYLE_FONT_SIZE to StyleFont Enum class → Remove unneeded constants for Aural CSS from nsStyleConsts.h
Priority: -- → P3
Should we get the r+'d patch landed? (or was there any more that needs to be done here?)
Status: UNCONFIRMED → NEW
status-firefox57: affected → wontfix
status-firefox58: --- → affected
Ever confirmed: true
Flags: needinfo?(akriti.v10)
Flags: needinfo?(manishearth)
Flags: needinfo?(manishearth)
Flags: needinfo?(akriti.v10)
Pushed by manishearth@gmail.com: https://hg.mozilla.org/integration/autoland/rev/5a63d8457a2a Removed unwanted constants from nsStyleconsts.h r=manishearth
https://hg.mozilla.org/mozilla-central/rev/5a63d8457a2a
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox58: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: