See test-case, long-tap on selected text and see no magnifying glass in text-selection action-bar. Attempt to long-tap on selected text again, on second attempt the magnifying glass will appear. -- Nightly (12/06) | LG Nexus 4 (Android 4.4)

Not sure if sriram is back or not, so I figured you might enjoy learning this margaret (plus, its good to spread the knowledge out :) ) Found a few little bugs here :( We're sending the correct data to Java, but our measurements say we don't have room in the action bar for this item. We bail and didn't show the menu button. 1.) This fixes the menu button showing but making sure we DON'T add these to GeckoMenu.mActionItems (so that GeckoMenu.hasVisibleItems returns true). 2.) Only adjust the width for a menu button if the menu button isn't visible. 3.) Fix removing items to use the measured with with a View.UNSPECIFIED spec. That's the measurement we use when adding items. It won't match the size after the items are added (but gives us a good maximum). We need to use the same spec when items are removed.

Comment on attachment 8344829 [details] [diff] [review] Patch Review of attachment 8344829 [details] [diff] [review]: ----------------------------------------------------------------- It took me a bit of time to learn the context around this, and I'm still not 100% confident I understand how everything works, but this seems reasonable to me given your explanation. However, I think it would also be good for Sriram to take a look at this if there's not a big rush to get this landed today.

Comment on attachment 8344829 [details] [diff] [review] Patch Lets have ram take a look then :)

Review ping?

Comment on attachment 8344829 [details] [diff] [review] Patch Review of attachment 8344829 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. ::: mobile/android/base/menu/GeckoMenu.java @@ +174,5 @@ > + if (mActionItemBarPresenter.addActionItem(actionView)) { > + mActionItems.put(menuItem, actionView); > + mItems.add(menuItem); > + return true; > + } A newline after this.

Reviewed.

Comment on attachment 8344829 [details] [diff] [review] Patch [Approval Request Comment] Bug caused by (feature/regressing bug #): 768667 User impact if declined: icons randomly don't appear in the menu Testing completed (on m-c, etc.): Landed on mc today Risk to taking this patch (and alternatives if risky): Low risk. Fixing a new feature. Without, will have to disable. String or IDL/UUID changes made by this patch: None

Verified fixed on: Build: Firefox for Android 28 Beta 8 Device: LG Nexus 4 OS: Android 4.4