Closed Bug 625792 Opened 9 years ago Closed 9 years ago

GetDeviceOrientation (-moz-device-orientation) does not work right on Android

Categories

(Core :: Widget: Android, defect)

All
Android
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla2.0b10
Tracking Status
fennec 2.0b4+ ---

People

(Reporter: mbrubeck, Assigned: mbrubeck)

References

Details

(Whiteboard: [has-patch][not-fennec-11])

Attachments

(1 file, 1 obsolete file)

-moz-device-orientation is supposed to use the screen dimensions rather than the window size.  However, nsScreenAndroid::GetRect uses nsWindow::GetAndroidBounds which uses the window size instead.
Attached patch patch (obsolete) — Splinter Review
Attachment #503894 - Flags: review?(blassey.bugs)
blocking. this makes the UX in tablets not-so-good.
tracking-fennec: --- → 2.0b4+
Comment on attachment 503894 [details] [diff] [review]
patch


Ouch. This part in the event handler needs to be flattened. Might as well while we're in here. The rest looks fine.

>+            if (newScreenWidth != gAndroidScreenBounds.width ||
>+                newScreenHeight != gAndroidScreenBounds.height) {
if (newScreenWidth == gAndroidScreenBounds.width &&
    newScreenHeight == gAndroidScreenBounds.height)
    break;

>+
>+                gAndroidScreenBounds.width = newScreenWidth;
>+                gAndroidScreenBounds.height = newScreenHeight;
> 
> #ifdef MOZ_IPC
>-            if (XRE_GetProcessType() == GeckoProcessType_Default) {
>-                if (!gContentCreationNotifier) {
>-                    nsCOMPtr<nsIObserverService> obs =
>-                        do_GetService("@mozilla.org/observer-service;1");
>-                    if (obs) {
>-                        nsCOMPtr<ContentCreationNotifier> notifier = new ContentCreationNotifier;
>-                        if (NS_SUCCEEDED(obs->AddObserver(notifier, "ipc:content-created", PR_FALSE))) {
>-                            if (NS_SUCCEEDED(obs->AddObserver(notifier, "xpcom-shutdown", PR_FALSE)))
>-                                gContentCreationNotifier = notifier;
>-                            else {
>-                                obs->RemoveObserver(notifier, "ipc:content-created");
>+                if (XRE_GetProcessType() == GeckoProcessType_Default) {

if (XRE_GetProcessType() != GeckoProcessType_Default)
    break;

Lets move the cp->SendScreenSizeChanged(gAndroidScreenBounds); code up here and then..

>+                    if (!gContentCreationNotifier) {

if (gContentCreationNotifier)
    break;

>+                        nsCOMPtr<nsIObserverService> obs =
>+                            do_GetService("@mozilla.org/observer-service;1");
>+                        if (obs) {

if (!obs)
    break;

>+                            nsCOMPtr<ContentCreationNotifier> notifier = new ContentCreationNotifier;
>+                            if (NS_SUCCEEDED(obs->AddObserver(notifier, "ipc:content-created", PR_FALSE))) {

if (NS_FAILED(obs->AddObserver(notifier, "ipc:content-created", PR_FALSE)))
    break;

>+                                if (NS_SUCCEEDED(obs->AddObserver(notifier, "xpcom-shutdown", PR_FALSE)))
>+                                    gContentCreationNotifier = notifier;
>+                                else {
>+                                    obs->RemoveObserver(notifier, "ipc:content-created");
>+                                }
>                             }
>                         }
>                     }
>+                    ContentParent *cp = ContentParent::GetSingleton(PR_FALSE);
>+                    if (cp)
>+                        unused << cp->SendScreenSizeChanged(gAndroidScreenBounds);
>                 }
>-                ContentParent *cp = ContentParent::GetSingleton(PR_FALSE);
>-                if (cp)
>-                    unused << cp->SendScreenSizeChanged(gAndroidBounds);
>+#endif
>             }
>-#endif
>             break;
>         }
>
Attachment #503894 - Flags: review?(blassey.bugs) → review+
Comment on attachment 503894 [details] [diff] [review]
patch

The SIZE_CHANGED android event should be for a change in window size. You should either create a new event for the screen size changing, or (if this isn't called often) have GetAndroidScreenBounds() make a jni call to get the screen metrics.
Attachment #503894 - Flags: review+ → review-
Attachment #503894 - Flags: review- → review+
With mwu's comments fixed.
Attachment #503894 - Attachment is obsolete: true
Attachment #503947 - Flags: review+
Keywords: checkin-needed
Whiteboard: [has-patch]
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b10
Blocks: 625940
Duplicate of this bug: 625940
Verified fix on Mozilla/5.0 (Android; Linux armv71; rv:2.0b9pre) Gecko/20110116 Firefox/4.0b9pre Fennec/4.0b4pre
Status: RESOLVED → VERIFIED
Whiteboard: [has-patch] → [has-patch][not-fennec-11]
You need to log in before you can comment on or make changes to this bug.