575206 - Incorrect accelerometer platform defines in nsLayoutModule.cpp

Status: NEW

(Firefox Build System :: General, defect)

Description

[Michael Wu [:mwu]]

Bug 562181 added a bunch of #ifdefs that look like:

+#if defined(XP_UNIX)    || \
+    defined(_WINDOWS)   || \
+    defined(machintosh) || \
+    defined(android)

machintosh is obviously incorrect, android should be capitalized, and I suspect _WINDOWS would do better as XP_WIN.

Also, android and machintosh are likely not necessary since they are both unixish platforms.

Comment 1

[Mike Kristoffersen (:MikeK)]

Attachment: Like this?

Comment 2

[Michael Wu [:mwu]]

Comment on attachment 454474 [details] [diff] [review]
Like this?

Looks right to me. Hopefully there aren't any platforms that define XP_UNIX that we don't support accelerometer on.

Comment 3

[(not currently active) Ted Mielczarek]

I suspect there are plenty. Things like Solaris, *BSD, etc. Should we instead be testing MOZ_WIDGET_GTK2 || MOZ_WIDGET_QT there?

Comment 4

[Michael Wu [:mwu]]

(In reply to comment #3)
> I suspect there are plenty. Things like Solaris, *BSD, etc. Should we instead
> be testing MOZ_WIDGET_GTK2 || MOZ_WIDGET_QT there?
What other widget backend would Solaris/*BSD use?

Comment 5

[(not currently active) Ted Mielczarek]

Yeah, true. Are we feature testing for something for accelerometer support on GTK2?

Comment 6

[Mike Kristoffersen (:MikeK)]

I think testing MOZ_WIDGET_GTK2 || MOZ_WIDGET_QT will be ok, as the corresponding test in the make file is: 
ifneq (,$(filter qt gtk2,$(MOZ_WIDGET_TOOLKIT)))

What should the corresponding values be for mac and android - so it fits the make file:
ifneq (,$(filter cocoa,$(MOZ_WIDGET_TOOLKIT)))
and
ifneq (,$(filter android,$(MOZ_WIDGET_TOOLKIT)))

???

Comment 7

[(not currently active) Ted Mielczarek]

There's a MOZ_WIDGET_COCOA define, I don't know about android.