Closed Bug 1300337 Opened 4 years ago Closed 4 years ago
_ with None in style enums
58 bytes, text/x-review-board-request
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.
Let's see what try says about linux
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 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.
Attachment #8787899 - Flags: review+
I'm not a official peer to grant r+ though. heycam, you might want to take a look in case I miss something.
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.
Attachment #8787899 - Flags: review?(cam) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/fd597b8d1dd6 Replace None_ variants from nsStyleConsts.h with None; r=heycam,TYLin
You need to log in before you can comment on or make changes to this bug.