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)

ARM
Android
defect
Not set
normal

Tracking

(firefox30 verified, firefox31 verified, firefox32 verified)

VERIFIED FIXED
Firefox 31
Tracking Status
firefox30 --- verified
firefox31 --- verified
firefox32 --- verified

People

(Reporter: liuche, Assigned: wesj)

Details

Attachments

(2 files)

On tablets, the "Share" item in the context menu for a long-pressed link shows just the icon, not text.
Attached patch PatchSplinter Review
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)
https://hg.mozilla.org/mozilla-central/rev/a5cee95d219e
Assignee: nobody → wjohnston
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Comment on attachment 8407853 [details] [diff] [review]
Patch

Review of attachment 8407853 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm!
Attachment #8407853 - Flags: review?(liuche) → review+
Wes, looks like you put the wrong bug number in the bug from comment #2. Reopening this bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/9e163c842e3b
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Does this need to be uplifted to Fx30? If so, let's get some testing on it first.
Keywords: qawanted
Reproducible on Firefox for Android 30 Beta 1 on Asus Transformer TF (Android 4.0)
Wes - Uplift to Fx30?
Flags: needinfo?(wjohnston)
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)
Attachment #8407853 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(teodora.vermesan)
Keywords: verifyme
"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)
Status: RESOLVED → VERIFIED
Keywords: qawanted, verifyme
Reproducible on Samsung Galaxy Tab (Android 4.0.4) on 30 Beta 8. Uplift to Beta?
Flags: needinfo?(wjohnston)
Flags: needinfo?(mark.finkle)
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 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?
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+
Verified as fixed in build 30 beta 10 using Asus Transformer Pad TF300T (Android 4.2.1)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.