implement device motion - compassneedscalibration support

RESOLVED FIXED in mozilla14

Status

()

Core
Geolocation
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dougt, Assigned: dougt)

Tracking

unspecified
mozilla14
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Created attachment 604310 [details] [diff] [review]
v.1

implement compassneedscalibration support on android
Attachment #604310 - Flags: review?(josh)

Comment 1

6 years ago
Comment on attachment 604310 [details] [diff] [review]
v.1

Review of attachment 604310 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine to me.

::: dom/system/nsDeviceMotion.cpp
@@ +49,5 @@
>  #include "nsIServiceManager.h"
>  #include "nsIPrefService.h"
>  #include "nsDOMDeviceMotionEvent.h"
>  
> +#include <android/log.h>

#ifdef?

@@ +256,5 @@
>  
> +NS_IMETHODIMP
> +nsDeviceMotion::NeedsCalibration()
> +{
> +  __android_log_print(ANDROID_LOG_ERROR, "Gecko", "### FireNeedsCalibration - NeedsCalibration ");

I expect this needs some kind of ifdef goop for non-Android?

@@ +261,5 @@
> +
> +  if (!mEnabled)
> +    return NS_ERROR_NOT_INITIALIZED;
> +
> +  for (PRUint32 i = mListeners.Count(); i > 0 ; ) {

Any particular reason these loops need to go backwards?

@@ +270,5 @@
> +  for (PRUint32 i = mWindowListeners.Length(); i > 0 ; ) {
> +    --i;
> +
> +    // check to see if this window is in the background.  if
> +    // it is, don't send any device motion to it.

I think it's worth moving this common check into a helper.

@@ +294,5 @@
> +nsDeviceMotion::FireNeedsCalibration(nsIDOMDocument *domdoc,
> +				    nsIDOMEventTarget *target)
> +{
> +
> +  __android_log_print(ANDROID_LOG_ERROR, "Gecko", "### FireNeedsCalibration ");

#ifdef, I presume.

@@ +297,5 @@
> +
> +  __android_log_print(ANDROID_LOG_ERROR, "Gecko", "### FireNeedsCalibration ");
> +
> +  nsCOMPtr<nsIDOMEvent> event;
> +  bool defaultActionEnabled = true;

Move this down by where it's used.
Attachment #604310 - Flags: review?(josh) → review+
(Assignee)

Comment 2

6 years ago
I removed the logging code.  It really isn't needed.

> I think it's worth moving this common check into a helper.

yes!  I am fixing this up in another patch.  You'll get the review request soon, i hope!.
Assignee: nobody → doug.turner

Comment 3

6 years ago
Sorry, backed out for OS X M1/M3 oranges:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=c619bbb2951f
https://tbpl.mozilla.org/php/getParsedLog.php?id=10039398&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=10039239&tree=Mozilla-Inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/ec55dae77b79
https://hg.mozilla.org/mozilla-central/rev/781e874d0ff5
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.