1300337 - Replace None_ with None in style enums

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Manish Goregaokar [:manishearth]]

In bug 1277133 we started using enum classes in nsStyleConsts.h. Due to a `#define None` in X11 headers, we had to replace None variants with None_. Now that this has been fixed (bug 1288686), we can revert this.

Comment 1

[Manish Goregaokar [:manishearth]]

Attachment: Bug 1300337 - Replace None_ variants from nsStyleConsts.h with None;

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

Comment 2

[Manish Goregaokar [:manishearth]]

Let's see what try says about linux

Comment 3

[Manish Goregaokar [:manishearth]]

Comment on attachment 8787899 [details]
Bug 1300337 - Replace None_ variants from nsStyleConsts.h with None;

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

Comment 4

[Manish Goregaokar [:manishearth]]

Comment on attachment 8787899 [details]
Bug 1300337 - Replace None_ variants from nsStyleConsts.h with None;

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

Comment 5

[Manish Goregaokar [:manishearth]]

Comment on attachment 8787899 [details]
Bug 1300337 - Replace None_ variants from nsStyleConsts.h with None;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/76500/diff/3-4/

Comment 6

[Manish Goregaokar [:manishearth]]

Comment on attachment 8787899 [details]
Bug 1300337 - Replace None_ variants from nsStyleConsts.h with None;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/76500/diff/4-5/

Comment 7

[Manish Goregaokar [:manishearth]]

I removed the conditional inclusion of X11UndefineNone.h, the `#ifdef None` should suffice. Otherwise cases where X11UndefineNone got included both before and after X11 is don't work.

Comment 8

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Comment on attachment 8787899 [details]
Bug 1300337 - Replace None_ variants from nsStyleConsts.h with None;

https://reviewboard.mozilla.org/r/76500/#review76142

This looks good to me, but you'll need rebase to include the change made in bug 1297306 which added StyleClear::None_.

::: dom/plugins/ipc/PluginInstanceChild.cpp:66
(Diff revision 5)
>  
>  #include "mozilla/widget/WinMessages.h"
>  #include "mozilla/widget/WinModifierKeyState.h"
>  #include "mozilla/widget/WinNativeEventData.h"
>  #include "nsWindowsDllInterceptor.h"
>  

Nit: Extra blank line here.

Comment 9

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

I'm not a official peer to grant r+ though. heycam, you might want to take a look in case I miss something.

Comment 10

[Cameron McCormack (:heycam)]

Comment on attachment 8787899 [details]
Bug 1300337 - Replace None_ variants from nsStyleConsts.h with None;

https://reviewboard.mozilla.org/r/76500/#review76146

Looks good, although I wonder if we should instead be #including "X11None.h" after every #include "X11.h" so we don't have to use X11UndefineNone.h in every file we want to use None as an identifier.  I guess we might include some other e.g. GTK+ header that includes it and we won't realise we need to, though, so OK.

Comment 11

[Manish Goregaokar [:manishearth]]

Comment on attachment 8787899 [details]
Bug 1300337 - Replace None_ variants from nsStyleConsts.h with None;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/76500/diff/5-6/

Comment 12

[Manish Goregaokar [:manishearth]]

Comment on attachment 8787899 [details]
Bug 1300337 - Replace None_ variants from nsStyleConsts.h with None;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/76500/diff/6-7/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd597b8d1dd6
Replace None_ variants from nsStyleConsts.h with None; r=heycam,TYLin

Comment 14

[Phil Ringnalda (:philor)]

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