1386326 - Remove unneeded constants for Aural CSS from nsStyleConsts.h

Status: RESOLVED FIXED

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

Description

[akriti verma]

Attachment: nsStyleConsts.h

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

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 2

[akriti verma]

Attachment: structs_release.rs

Comment 3

[akriti verma]

Attachment: structs_debug.rs

Comment 4

[Emilio Cobos Álvarez (:emilio)]

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!

Comment 5

[akriti verma]

Attachment: nsStyleConsts.h

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 8892815 [details] [diff] [review]
nsStyleConsts.h

Please see the above link about how to produce a patch.

Comment 7

[akriti verma]

Attachment: Bug 1386326 - Removed unwanted constants from nsStyleconsts.h

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

Comment 8

[Manish Goregaokar [: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.

Comment 9

[akriti verma]

Attachment: Constants changed to enum classes

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(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.

Comment 11

[akriti verma]

Attachment: rb169734.patch

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

Comment 12

[Manish Goregaokar [: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.

Comment 13

[akriti verma]

Comment on attachment 8895937 [details]
Bug 1386326 - Removed unwanted constants from nsStyleconsts.h

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

Comment 14

[Manish Goregaokar [:manishearth]]

Comment on attachment 8895937 [details]
Bug 1386326 - Removed unwanted constants from nsStyleconsts.h

https://reviewboard.mozilla.org/r/167210/#review177448

Comment 15

[Daniel Holbert [:dholbert]]

Should we get the r+'d patch landed? (or was there any more that needs to be done here?)

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5a63d8457a2a
Removed unwanted constants from nsStyleconsts.h r=manishearth

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5a63d8457a2a