Closed
Bug 734325
Opened 12 years ago
Closed 12 years ago
implement device motion - compassneedscalibration support
Categories
(Core :: DOM: Geolocation, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: dougt, Assigned: dougt)
Details
Attachments
(1 file)
14.13 KB,
patch
|
jdm
:
review+
|
Details | Diff | Splinter Review |
implement compassneedscalibration support on android
Attachment #604310 -
Flags: review?(josh)
Comment 1•12 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•12 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•12 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
Comment 4•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/781e874d0ff5
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in
before you can comment on or make changes to this bug.
Description
•