Closed Bug 717681 Opened 11 years ago Closed 11 years ago

Horizontal band of grey band pixels invading URL bar

Categories

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

ARM
Android
defect

Tracking

(firefox11 verified, firefox12 verified, firefox13 verified, fennec11+)

VERIFIED FIXED
Firefox 12
Tracking Status
firefox11 --- verified
firefox12 --- verified
firefox13 --- verified
fennec 11+ ---

People

(Reporter: aaronmt, Assigned: sriram)

References

Details

Attachments

(3 files, 1 obsolete file)

Attached image 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
Attached image screen shot
I see this in ICS as well. More noticeable in HDPI.
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.
Assignee: nobody → sriram
tracking-fennec: --- → 11+
Priority: -- → P3
Attached patch Patch (obsolete) — Splinter Review
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.
Attachment #591075 - Flags: review?(mark.finkle)
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
Attachment #591075 - Flags: review?(mark.finkle) → review-
Attached patch PatchSplinter Review
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.
Attachment #591075 - Attachment is obsolete: true
Attachment #592290 - Flags: review?(mark.finkle)
Attachment #592290 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/cfa6e7eff71f
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 12
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.
Attachment #592290 - Flags: approval-mozilla-aurora?
Comment on attachment 592290 [details] [diff] [review]
Patch

[Triage Comment]
Mobile only - approved for Beta 11.
Attachment #592290 - Flags: approval-mozilla-aurora? → approval-mozilla-beta+
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
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.