Closed
Bug 1148390
Opened 10 years ago
Closed 10 years ago
Share icon from action bar is misaligned on gingerbread devices
Categories
(Firefox for Android Graveyard :: Theme and Visual Design, defect)
Tracking
(firefox36 unaffected, firefox37 unaffected, firefox38+ verified, firefox39+ verified, firefox40 verified, fennec38+)
VERIFIED
FIXED
Firefox 40
Tracking | Status | |
---|---|---|
firefox36 | --- | unaffected |
firefox37 | --- | unaffected |
firefox38 | + | verified |
firefox39 | + | verified |
firefox40 | --- | verified |
fennec | 38+ | --- |
People
(Reporter: cos_flaviu, Assigned: mcomella)
References
Details
Attachments
(4 files, 1 obsolete file)
Environment:
Device: Samsung Galaxy R (Android 2.3.4);
Build: Nightly 39.0a1 (2015-03-26);
Steps to reproduce:
1. Load a page (e.g.: en.wikipedia.org/wiki/Mozilla);
2. Long tap on a word.
Expected result:
The icons form action bar are correctly aligned.
Actual result:
Share icon is misaligned.
Notes:
Please check the attached screenshot.
Comment 1•10 years ago
|
||
[Tracking Requested - why for this release]: regression/polish
Comment 2•10 years ago
|
||
Tracking for 38 and 39 since this is a regression.
Sylvestre, I figured you would probably want to track and maybe take this in early beta if there is a simple fix.
Updated•10 years ago
|
tracking-fennec: ? → 38+
Comment 3•10 years ago
|
||
Capellla, have you touched this action bar recently? Would you be interested in taking this bug?
Flags: needinfo?(markcapella)
Keywords: regressionwindow-wanted
Comment 4•10 years ago
|
||
I can look. I've reviewed the code on a number of occasions, but I haven't changed anything. Is it actually a regression (tied to recent share-icon resource changes?), or a specific Gingerbread thing?
Flags: needinfo?(markcapella)
Comment 5•10 years ago
|
||
ping mcomella: ... is this related at all to Bug 1116910 ?
Flags: needinfo?(michael.l.comella)
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Mark Capella [:capella] from comment #5)
> ping mcomella: ... is this related at all to Bug 1116910 ?
I think it is to blame, yes - we replaced the assets there to work better on tablet, but it seems it's missing some style elements for GB. I wonder if there is an easy way to change the styles in the ActionBar?
Mark, if you want to take a stab, please go ahead, but I'll jump on this when I finish fixing up a few more regressions. :P
Flags: needinfo?(michael.l.comella)
Assignee | ||
Comment 7•10 years ago
|
||
Looks like the problem is because the button housing the share icon does not fill the space (see screenshot). Notably, the other assets have padding in the image so that they fill the space.
> GB doesn't have this problem depsite the padding - perhaps the size of the buttons are explicitly set?
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → michael.l.comella
Reporter | ||
Updated•10 years ago
|
status-firefox40:
--- → affected
Comment 8•10 years ago
|
||
Michael, are you going to fix this issue during the 38 cycle? Thanks
Flags: needinfo?(michael.l.comella)
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Sylvestre Ledru [:sylvestre] from comment #8)
> Michael, are you going to fix this issue during the 38 cycle? Thanks
Trying, but I think it's low priority and wouldn't mind a slip to 39.
tracking-fennec: 38+ → ?
Flags: needinfo?(michael.l.comella)
Assignee | ||
Comment 10•10 years ago
|
||
Looks like we use menuItemActionModeStyle [1] = GeckoActionBar.Button for the individual items. I tried setting explicit heights and widths to no avail - probably a quality of the underlying ImageButton - I'm not sure why v11+ works correctly though.
I noticed I can change padding to reduce the size of the images (and perhaps can make all of the icons the same size by using that), but I think the proper solution is to remove the whitespace in the other images, ala bug 1122752.
[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/TextSelection.java?rev=1ee3f4cdae47#280
Assignee | ||
Comment 11•10 years ago
|
||
Assignee | ||
Comment 12•10 years ago
|
||
/r/7019 - Bug 1148390 - Dynamically add padding to share icon on GB devices. r=capella
Pull down this commit:
hg pull -r 4750f2deab2340d26ef2d4a86df35c0aefef1ea5 https://reviewboard-hg.mozilla.org/gecko/
Attachment #8591973 -
Flags: review?(markcapella)
Comment 13•10 years ago
|
||
Comment on attachment 8591973 [details]
MozReview Request: bz://1148390/mcomella
This looks fine to me, but I don't have a GB device to test properly. Can you run this past wesj for proper approval?
Attachment #8591973 -
Flags: review?(markcapella) → feedback+
Assignee | ||
Updated•10 years ago
|
Attachment #8591973 -
Flags: review?(wjohnston)
Updated•10 years ago
|
Attachment #8591973 -
Flags: review?(wjohnston) → review+
Assignee | ||
Comment 14•10 years ago
|
||
Assignee | ||
Comment 15•10 years ago
|
||
Comment on attachment 8591973 [details]
MozReview Request: bz://1148390/mcomella
Approval Request Comment
[Feature/regressing bug #]: bug 1116910
[User impact if declined]:
Users on GB devices see a mis-aligned share icon in the action bar (which appears when long-press selecting text)
[Describe test coverage new/current, TreeHerder]:
Tested locally on my Android L N4 and a GB device
[Risks and why]:
Low - dynamically change the padding on only the share icon on GB. Worst case, we incorrectly check the device configuration and we apply this change to other devices. We can also get the padding amount incorrect, particularly on different device configurations (e.g. different screen sizes) but I think it should be the same across devices.
[String/UUID change made/needed]: None
Attachment #8591973 -
Flags: approval-mozilla-beta?
Attachment #8591973 -
Flags: approval-mozilla-aurora?
Comment 16•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Comment 17•10 years ago
|
||
Comment on attachment 8591973 [details]
MozReview Request: bz://1148390/mcomella
Should be in 38 beta 6.
Attachment #8591973 -
Flags: approval-mozilla-beta?
Attachment #8591973 -
Flags: approval-mozilla-beta+
Attachment #8591973 -
Flags: approval-mozilla-aurora?
Attachment #8591973 -
Flags: approval-mozilla-aurora+
Comment 18•10 years ago
|
||
Comment 19•10 years ago
|
||
Comment 20•10 years ago
|
||
Verified as fixed using:
Device: HTC Desire HD (Android 2.3.5)
Builds: Firefox for Android 39.0a2 (2015-04-16) and Firefox for Android 40.0a1 (2015-04-16)
Updated•10 years ago
|
tracking-fennec: ? → 38+
Reporter | ||
Comment 21•10 years ago
|
||
Verified as fixed in Firefox 38 Beta 8 build 3;
Device: Samsung Galaxy R (Android 2.3.4).
Reporter | ||
Updated•10 years ago
|
Keywords: regressionwindow-wanted
Reporter | ||
Updated•10 years ago
|
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 22•10 years ago
|
||
Attachment #8591973 -
Attachment is obsolete: true
Attachment #8619889 -
Flags: review+
Assignee | ||
Comment 23•10 years ago
|
||
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
•