Closed
Bug 730775
Opened 13 years ago
Closed 13 years ago
Grey bar underneath the awesomebar on tablets
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(blocking-fennec1.0 -)
RESOLVED
FIXED
Firefox 15
Tracking | Status | |
---|---|---|
blocking-fennec1.0 | --- | - |
People
(Reporter: glandium, Assigned: sriram)
References
Details
(Keywords: uiwanted, Whiteboard: [tablet])
Attachments
(2 files, 1 obsolete file)
213.18 KB,
image/png
|
Details | |
72.50 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
See attached screenshot. The bottom part of the background image looks weird, and the right part, for the menu, is disconnected. This is on linaro android based on ICS.
Reporter | ||
Comment 1•13 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #0) > See attached screenshot. The bottom part of the background image looks > weird In fact it looks like it's repeated vertically. FWIW I expect the same to happen on the TF700 (the next ASUS Transformer, which is supposed to have a Full HD display) due in june.
Assignee | ||
Comment 2•13 years ago
|
||
We don't have resources ready for tablets yet. So waiting for the tablet UI designs and resources to fix this issue.
Keywords: uiwanted
Reporter | ||
Comment 3•13 years ago
|
||
So, it turns out this also happens at 1280x800 on the asus transformer. So all it would take is a higher density on phones for that to happen.
Comment 4•13 years ago
|
||
This is tablet work. Out of scope for the initial release.
blocking-fennec1.0: --- → -
Updated•13 years ago
|
Summary: awesomebar tile looks bad in 1080p → Grey bar underneath the awesomebar on tablets
Updated•13 years ago
|
tracking-fennec: --- → ?
Updated•13 years ago
|
Assignee: nobody → sriram
Reporter | ||
Comment 6•13 years ago
|
||
"Grey bar underneath the awesomebar on tablets" is not the whole story, there's also the gap on the tab button. I'm curious also, doesn't it happen on the Google Nexus in landscape mode (which has the same horizontal resolution as the ASUS Transformer, which does it)
Comment 7•13 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #6) > "Grey bar underneath the awesomebar on tablets" is not the whole story, > there's also the gap on the tab button. This is bug 737479. > I'm curious also, doesn't it happen on the Google Nexus in landscape mode > (which has the same horizontal resolution as the ASUS Transformer, which > does it) No, because the Galaxy Nexus has a higher pixel density so it uses different layout and assets. The various layout bugs on tablets should be fixed when we start working on the layout and assets for the tablet UI.
Assignee | ||
Comment 10•13 years ago
|
||
The action-bar in tablets use a 56dip height for both portrait and landscape. Our current resources are 40dip (landscape) and 48dip (portrait). This brings the grey bar in tablets. This patch resolves it by adding few more folders that qualify for tablets. The resources are just scaled up as place holders until we get the actual resources from UX team. drawable-sw600dp-*/ is for supporting 7" tablets. We can drop that folder, if we don't want to support 7" tablets. However the action-bar in 7" tablets will have 56dip, which will cause this issue there again.
Attachment #625726 -
Flags: review?(mark.finkle)
Comment 11•13 years ago
|
||
Comment on attachment 625726 [details] [diff] [review] WIP feedback+ Waiting for the real PNGs before r+. The size of address_bar_texture_tablet.png seems quite large. Is there anyway to reduce the filesize a bit? I'd like to keep the APK size as small as we can, since we are adding other things, like locales.
Attachment #625726 -
Flags: review?(mark.finkle) → feedback+
Assignee | ||
Comment 12•13 years ago
|
||
This patch replaces the resources from the previous patch. This is a bug in honeycomb that causes the background of action-bar not to repeat. This has to be done in code manually. This patch fixes that too. (For a brief second, the urlbar is corrupted. This is for the time when android loads it from xml, and we do initialize(). We cannot do anything about it. However, this will be fixed as a part of bug 739355, which replaces the entire browser's url-bar for tablets).
Attachment #625726 -
Attachment is obsolete: true
Attachment #626674 -
Flags: review?(mark.finkle)
Comment 13•13 years ago
|
||
Comment on attachment 626674 [details] [diff] [review] Patch >diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java >+ public boolean isTablet() { >+ DisplayMetrics metrics = new DisplayMetrics(); >+ getWindowManager().getDefaultDisplay().getMetrics(metrics); >+ >+ // Checking for sw600dp for 7" tablet. >+ if (((metrics.widthPixels / metrics.density) >= 600) && >+ ((metrics.heightPixels / metrics.density) >= 600)) >+ return true; >+ else >+ return false; >+ } I am not a big fan of needing to do this in Java code. It seems brittle and easily broken. We already have an isTablet method in GeckoAppShell too. > public void refreshActionBar() { >+ // Version 11 and 12 doesn't support tiling of bitmap in action bar. >+ if (Build.VERSION.SDK_INT == 11 || Build.VERSION.SDK_INT == 12) { >+ BitmapDrawable bitmap = new BitmapDrawable(BitmapFactory.decodeResource(getResources(), R.drawable.address_bar_texture_tablet)); >+ bitmap.setTileModeX(android.graphics.Shader.TileMode.REPEAT); >+ background = bitmap; Is this code removed in later patches (in bug 739355) ? >+ } else if (isTablet()) { >+ background = getResources().getDrawable(R.drawable.address_bar_bg); >+ } else { >+ background = getResources().getDrawable(R.drawable.gecko_actionbar_bg); >+ } We can't do this in layouts XML ? >+ // Version 11 & 12 doesn't support tiling of bitmaps in action bar. >+ // Refresh it to avoid corruption. >+ if (Build.VERSION.SDK_INT == 11 || Build.VERSION.SDK_INT == 12) >+ refreshActionBar(); Is this code removed in bug 739355 ? >diff --git a/mobile/android/base/Makefile.in b/mobile/android/base/Makefile.in >+RES_DRAWABLE_SW600DP_MDPI = \ >+ res/drawable-sw600dp-mdpi/address_bar_bg.xml \ >+ res/drawable-sw600dp-mdpi/address_bar_texture_tablet.png \ >+ $(NULL) >+ >+RES_DRAWABLE_SW600DP_HDPI = \ >+ res/drawable-sw600dp-hdpi/address_bar_texture_tablet.png \ >+ $(NULL) >+ >+RES_DRAWABLE_SW600DP_XHDPI = \ >+ res/drawable-sw600dp-xhdpi/address_bar_texture_tablet.png \ >+ $(NULL) I wonder if we should add a comment about these being for 7" tablets r+, but the big images and the Java code for tablets worries me. Also, I want to make sure we remove all the code we can when bug 739355 lands.
Attachment #626674 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 14•13 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #13) > Comment on attachment 626674 [details] [diff] [review] > Patch > > >diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java > > >+ public boolean isTablet() { > >+ DisplayMetrics metrics = new DisplayMetrics(); > >+ getWindowManager().getDefaultDisplay().getMetrics(metrics); > >+ > >+ // Checking for sw600dp for 7" tablet. > >+ if (((metrics.widthPixels / metrics.density) >= 600) && > >+ ((metrics.heightPixels / metrics.density) >= 600)) > >+ return true; > >+ else > >+ return false; > >+ } > > I am not a big fan of needing to do this in Java code. It seems brittle and > easily broken. We already have an isTablet method in GeckoAppShell too. Probably we should consolidate both and move to GeckoApplication. Since it's not going to change between activities, we can probably have it there. Or, we can have it in GeckoApp as its the base activity. Basically, it's better to have it UI thread, so all other UI calling it won't have "wrong thread exceptions". I would like to take this as a followup. > > > public void refreshActionBar() { > > >+ // Version 11 and 12 doesn't support tiling of bitmap in action bar. > >+ if (Build.VERSION.SDK_INT == 11 || Build.VERSION.SDK_INT == 12) { > >+ BitmapDrawable bitmap = new BitmapDrawable(BitmapFactory.decodeResource(getResources(), R.drawable.address_bar_texture_tablet)); > >+ bitmap.setTileModeX(android.graphics.Shader.TileMode.REPEAT); > >+ background = bitmap; > > Is this code removed in later patches (in bug 739355) ? This can be removed only after we add custom menu. Otherwise, the default menu button will have corrupted background. We can have a background for menu button given by android, but not for the overflow items like refresh and bookmark. > > >+ } else if (isTablet()) { > >+ background = getResources().getDrawable(R.drawable.address_bar_bg); > >+ } else { > >+ background = getResources().getDrawable(R.drawable.gecko_actionbar_bg); > >+ } > > We can't do this in layouts XML ? This needs to be set, reset everytime we rotate the phone in ICS. The action-bar background holds the tail for Nexus. This can also be removed with custom menu. > > > >+ // Version 11 & 12 doesn't support tiling of bitmaps in action bar. > >+ // Refresh it to avoid corruption. > >+ if (Build.VERSION.SDK_INT == 11 || Build.VERSION.SDK_INT == 12) > >+ refreshActionBar(); > > Is this code removed in bug 739355 ? This can be removed with tablet UI, if there is no corruption found. I'll try it and remove there. > > >diff --git a/mobile/android/base/Makefile.in b/mobile/android/base/Makefile.in > >+RES_DRAWABLE_SW600DP_MDPI = \ > >+ res/drawable-sw600dp-mdpi/address_bar_bg.xml \ > >+ res/drawable-sw600dp-mdpi/address_bar_texture_tablet.png \ > >+ $(NULL) > >+ > >+RES_DRAWABLE_SW600DP_HDPI = \ > >+ res/drawable-sw600dp-hdpi/address_bar_texture_tablet.png \ > >+ $(NULL) > >+ > >+RES_DRAWABLE_SW600DP_XHDPI = \ > >+ res/drawable-sw600dp-xhdpi/address_bar_texture_tablet.png \ > >+ $(NULL) > > I wonder if we should add a comment about these being for 7" tablets > > r+, but the big images and the Java code for tablets worries me. Also, I > want to make sure we remove all the code we can when bug 739355 lands.
Assignee | ||
Comment 15•13 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/b46ce2cea55b
Comment 16•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b46ce2cea55b
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
Updated•12 years ago
|
Whiteboard: [tablet]
Updated•11 years ago
|
tracking-fennec: ? → ---
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•