643301 - The notification about extensions becoming disabled appears half off screen

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Extension Compatibility, defect)

Description

[Dave Townsend [:mossop]]

Just updated to Fennec 4.1a1pre and it had to disable one of my extensions (URL Fixer). It displayed a notification about this on startup but it was half off the right of the screen so I couldn't really read it.

Comment 1

[Matt Brubeck (:mbrubeck)]

I'm seeing this too; seems to be Android-only.

Comment 2

[Matt Brubeck (:mbrubeck)]

The problem is in this code.  During Browser.startup(), window.innerWidth is not set correctly yet (it is equal to 1) so the calculation is wrong:

http://mxr.mozilla.org/mobile-browser/source/chrome/content/AlertsHelper.js#51

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: patch

Fixes the problem by moving the code to the UIReadyDelayed handler.  We don't want to show alerts before the UI is ready.

Also adds a warning to the alert service, in case anyone tries this again.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 521294 [details] [diff] [review]
patch


> #ifdef ANDROID
>     let offset = (window.innerWidth - container.width) / 2;
>-    if (container.hasAttribute("left"))
>+    if (offset < 0)
>+      Cu.reportError("showAlertNotification called before the window is ready");
>+    else if (container.hasAttribute("left"))
>       container.setAttribute("left", offset);
>     else
>       container.setAttribute("right", offset);

Can we add the new if (offset) check as a standalone if block, and leave the existing if block alone?

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #4)

> Can we add the new if (offset) check as a standalone if block, and leave the
> existing if block alone?

Or does the new if check actually have an impact on the position of the toaster alert?

Comment 6

[Matt Brubeck (:mbrubeck)]

(In reply to comment #5)
> Or does the new if check actually have an impact on the position of the toaster
> alert?

The new "if" check prevents the toaster alert from ending up off-screen.  (If someone else makes this mistake, it'll end up with the Maemo-style positioning instead.)

Comment 7

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/mobile-browser/rev/03227d0d7fc3

Comment 8

[Andreea Pod]

Verified on build: Mozilla /5.0 (Android;Linux armv7l;rv:2.2a1pre)
Gecko/20110401 Firefox/4.2a1pre Fennec /4.1a1pre 
Device: HTC Desire (Android 2.2)