Closed
Bug 997406
Opened 10 years ago
Closed 10 years ago
Tablet: "Share" in context menu for link shows icon not text
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox30 verified, firefox31 verified, firefox32 verified)
VERIFIED
FIXED
Firefox 31
People
(Reporter: liuche, Assigned: wesj)
Details
Attachments
(2 files)
109.12 KB,
image/png
|
Details | |
1.39 KB,
patch
|
liuche
:
review+
Sylvestre
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
On tablets, the "Share" item in the context menu for a long-pressed link shows just the icon, not text.
Assignee | ||
Comment 1•10 years ago
|
||
This width check is in place because of the main menu, where we show Share and Bookmark side by side. We want to show this icon if Bookmark is present, but remove it if Bookmark is in the actionbar (on tablets). Chenxia's tablet has padding in the menu that throws off the calculation. This patch just forces us to also take into account padding on both the icon view and the parent row. Tested on the device and this fixes the bug.
Attachment #8407853 -
Flags: review?(liuche)
Comment 2•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a5cee95d219e
Assignee: nobody → wjohnston
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Reporter | ||
Comment 3•10 years ago
|
||
Comment on attachment 8407853 [details] [diff] [review] Patch Review of attachment 8407853 [details] [diff] [review]: ----------------------------------------------------------------- lgtm!
Attachment #8407853 -
Flags: review?(liuche) → review+
Reporter | ||
Comment 4•10 years ago
|
||
Wes, looks like you put the wrong bug number in the bug from comment #2. Reopening this bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/9e163c842e3b
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9e163c842e3b
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Comment 7•10 years ago
|
||
Does this need to be uplifted to Fx30? If so, let's get some testing on it first.
Comment 8•10 years ago
|
||
Reproducible on Firefox for Android 30 Beta 1 on Asus Transformer TF (Android 4.0)
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8407853 [details] [diff] [review] Patch [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 942270, Needs uplift to 30 (which may be beta soon?) User impact if declined: Menuitem looks strange on some devices. Testing completed (on m-c, etc.): Landed on mc a week ago. Seems good! Risk to taking this patch (and alternatives if risky): Low risk. Just takes into account some padding in a calculation. Most devices don't seem to have any padding here so it doesn't matter, but at least one manufacturer added some. String or IDL/UUID changes made by this patch: none.
Attachment #8407853 -
Flags: approval-mozilla-aurora?
Flags: needinfo?(wjohnston)
Updated•10 years ago
|
status-firefox31:
--- → affected
status-firefox32:
--- → fixed
Updated•10 years ago
|
Attachment #8407853 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 11•10 years ago
|
||
"Share" item in the context menu for links shows the text, so: Verified fixed on Firefox for Android 31.0a2 and 32.0a1 (2014-05-04) on Asus Transformer TF (Android 4.0).
Flags: needinfo?(teodora.vermesan)
Updated•10 years ago
|
Updated•10 years ago
|
Comment 12•10 years ago
|
||
Reproducible on Samsung Galaxy Tab (Android 4.0.4) on 30 Beta 8. Uplift to Beta?
Flags: needinfo?(wjohnston)
Flags: needinfo?(mark.finkle)
Comment 13•10 years ago
|
||
I believe it's too late for Beta uplift. Sadly, this patch was approved for uplift to Fx30 back on April 30th, but it seems like it was forgotten.
Flags: needinfo?(mark.finkle)
Comment 14•10 years ago
|
||
Comment on attachment 8407853 [details] [diff] [review] Patch Same as the Aurora request above. This was a+ for Aurora after Aurora went to Beta, but the flags were not updated.
Attachment #8407853 -
Flags: approval-mozilla-beta?
Updated•10 years ago
|
status-firefox30:
--- → affected
Comment 15•10 years ago
|
||
Comment on attachment 8407853 [details] [diff] [review] Patch small fix, we can take this for our last Beta.
Attachment #8407853 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 16•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/19b1649f060a
Flags: needinfo?(wjohnston)
Comment 18•10 years ago
|
||
Verified as fixed in build 30 beta 10 using Asus Transformer Pad TF300T (Android 4.2.1)
Updated•3 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
•