717681 - Horizontal band of grey band pixels invading URL bar

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Aaron Train [:aaronmt]]

Attachment: screenshot

See screenshot.

Testing latest inbound, so I think this is a regression from bug 714767. Not sure if ICS only.

--
Samsung Nexus S (Android 4.0.3)
20120112091623
http://hg.mozilla.org/integration/mozilla-inbound/rev/cd17d9409fda

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

Attachment: screen shot

I see this in ICS as well. More noticeable in HDPI.

Comment 2

[Sriram Ramasubramanian [:sriram]]

There is some problem with orientation that is getting stored.
mOrientation flag in GeckoApp stores the last orientation. If the phone is in portrait mode, screen turned off, and turned on, the phone opens in landscape mode, the mOrientation is still in "portrait" as the app doesnt get the change in orientation event. Hence the resources are not refreshed during this change.

The bug fix of 714767 could have caused this regression. The fix for 714767 didn't fix the base problem there.

Comment 3

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

This patch tries to fix the ActionBar during onResume().
This can happen when the activity was started in landscape mode, gone to different activity and came back in portrait mode.
In normal rotations, onConfigurationChanged() takes care of refreshing the ActionBar.
For good readability, the code has been factored out a little bit.

Comment 4

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

Comment on attachment 591075 [details] [diff] [review]
Patch

>diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java

>         if (Build.VERSION.SDK_INT >= 11) {
>+            refreshActionBar();
>         } else {
>             mBrowserToolbar = (BrowserToolbar) findViewById(R.id.browser_toolbar);
>         }

I assume the (Build.VERSION.SDK_INT >= 11) is needed

>-            if (Build.VERSION.SDK_INT >= 11) {
>-                mBrowserToolbar = (BrowserToolbar) getLayoutInflater().inflate(R.layout.browser_toolbar, null);
>-
>-                Tab tab = Tabs.getInstance().getSelectedTab();
>-                if (tab != null) {
>-                    mBrowserToolbar.setTitle(tab.getDisplayTitle());
>-                    mBrowserToolbar.setFavicon(tab.getFavicon());
>-                    mBrowserToolbar.setSecurityMode(tab.getSecurityMode());
>-                    mBrowserToolbar.setProgressVisibility(tab.isLoading());
>-                    mBrowserToolbar.setShadowVisibility(!(tab.getURL().startsWith("about:")));
>-                    mBrowserToolbar.updateTabs(Tabs.getInstance().getCount());
>-                }
>-
>-                GeckoActionBar.setBackgroundDrawable(this, getResources().getDrawable(R.drawable.gecko_actionbar_bg));
>-                GeckoActionBar.setCustomView(mAppContext, mBrowserToolbar);
>-            }
>+            refreshActionBar();

Don't we need to keep the (Build.VERSION.SDK_INT >= 11) check?

>diff --git a/mobile/android/base/resources/drawable/address_bar_bg.xml b/mobile/android/base/resources/drawable/address_bar_bg.xml

>-            android:tileMode="mirror"
>+            android:tileMode="repeat"

We need to be sure this does not break bug 714767.

r- to decide on the version checks

Comment 5

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

This patch adds the check for Build version. This doesn't regress the other bug. The other bug wasn't fixed properly -- which requires taking care of orientation on onResume().

I have the check for Build version inside refreshActionBar() just to make sure someone else calling the function doesn't need to make explicit checks before calling this function.

Comment 6

[Sriram Ramasubramanian [:sriram]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/cfa6e7eff71f

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/cfa6e7eff71f

Comment 8

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 592290 [details] [diff] [review]
Patch

[Approval Request Comment]
User impact if declined: 
There will be a grey bar in the url-bar at times after rotation. This is need to avoid UI corruption in the url-bar.

Testing completed (on m-c, etc.): Landed on m-c on 01/31

Risk to taking this patch (and alternatives if risky):
This just adds refreshing the url-bar in onResume() also. The logic is left untouched.

String changes made by this patch: None.

Comment 9

[Alex Keybl [:akeybl]]

Comment on attachment 592290 [details] [diff] [review]
Patch

[Triage Comment]
Mobile only - approved for Beta 11.

Comment 10

[Sriram Ramasubramanian [:sriram]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c8d32ef74863

Comment 11

[Cristian Nicolae (:xti)]

Verified fixed on:

Firefox 13.0a1 (2012-02-06)
20120206031148
http://hg.mozilla.org/mozilla-central/rev/814d0b2dbaba
Device: HTC Desire Z
OS: Android 2.3.3

Firefox 12.0a2 (2012-02-06)
20120206042011
http://hg.mozilla.org/releases/mozilla-aurora/rev/9fb0c06ceb49
Device: HTC Desire Z
OS: Android 2.3.3

Firefox 11.0
20120206202409
http://hg.mozilla.org/releases/mozilla-beta/rev/1c0aba74d116
Device: HTC Desire Z
OS: Android 2.3.3