See the mocks and comments in Bug 1234319 - we've removed the "add to reading" list button which makes the main menu look rather odd. We'll want this in 48, which might require uplifting (but hopefully we can get this done this week).

We only care about API >= 14, so there's no need for the pre-v11 menu. Review commit: https://reviewboard.mozilla.org/r/47629/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47629/

We also need to shuffle our icon sizes around: our xhdpi icon size matches the hdpi icon sizes of the other icons in the top bar (modulo padding, which we need to add as part of this commit), similarly our xxhdpi icon is shifted to xhdpi. We don't supply an xxhdpi icon at all for any of the icons in the top bar of the menu, hence that size is removed (and reused for the xhdpi icon, see above). Review commit: https://reviewboard.mozilla.org/r/47631/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47631/

Here's a screenshot of the new menu - I'm not sure if we had a firm decision regarding removing the back button, given the telemetry data I'd be wary of removing it?

Comment on attachment 8743187 [details] MozReview Request: Bug 1265433 - Pre: remove v11 prefixes from menus r=sebastian https://reviewboard.mozilla.org/r/47629/#review44533

Comment on attachment 8743188 [details] MozReview Request: Bug 1265433 - Move bookmarks star into top bar of menu r?sebastian Something doesn't seem to be right with the "Share" menu item. After sharing the quick share button(s) show up but the "Share" item does not go away and the share icon does not appear next to the quick buttons - this seems to be wrong? I'll upload some screenshots.

Is this correct? Something is weird about having two share rows.

Comment on attachment 8743403 [details] Screenshot_20160420-194025.png This should not happen. I'll attach a mock in a bit but basically "Share" should be the default state. But once the user presses on some sharing options, we should be over riding that row with our usual app icons.

Looking closely at the screenshot in comment 6, there seems to be a weird 1 dp transparency around the edges of the menu. It seems to only be there for items below the first row. Do you see that? we should fix that too, it looks unfinished. :(

^looks like that work is being tracked in bug 1148552.

(In reply to Sebastian Kaspari (:sebastian) from comment #5) > Comment on attachment 8743188 [details] > MozReview Request: Bug 1265433 - Move bookmarks star into top bar of menu > r?sebastian > > Something doesn't seem to be right with the "Share" menu item. After sharing > the quick share button(s) show up but the "Share" item does not go away and > the share icon does not appear next to the quick buttons - this seems to be > wrong? > > I'll upload some screenshots. Huh, I was testing on an Android 4 device, and I never realised we're doing different things on newer versions (on 4 I get a submenu appearing when I select the share menu item). I'll need to investigate a bit more!

This is what it should look like with 4 items along the top. Will attach specs later.

Here's how the spacing is spec'd out. Icons should be the same and are just centered in the space. Does this make sense?

Here's the menu before a user "shares" a page. After which, we can cap the quick share as depicted in the earlier screenshot.

Review commit: https://reviewboard.mozilla.org/r/49253/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/49253/

Comment on attachment 8743187 [details] MozReview Request: Bug 1265433 - Pre: remove v11 prefixes from menus r=sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47629/diff/1-2/

Comment on attachment 8743188 [details] MozReview Request: Bug 1265433 - Move bookmarks star into top bar of menu r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47631/diff/1-2/

Comment on attachment 8743188 [details] MozReview Request: Bug 1265433 - Move bookmarks star into top bar of menu r?sebastian I think I've now managed to build the right behaviour, I'm going to try and figure out if we can simplify this code more though. The code seems extremely unobvious (it needed to be more abstract for the previous menu implementation), however I'm not sure how much work simplifying it would actually be.

Can you share a screenshot in the meantime? :)

Comment on attachment 8746094 [details] MozReview Request: Bug 1265433 - Pre: remove unnecessary v11 checks r=me Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49253/diff/1-2/

Comment on attachment 8743187 [details] MozReview Request: Bug 1265433 - Pre: remove v11 prefixes from menus r=sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47629/diff/2-3/

Comment on attachment 8743188 [details] MozReview Request: Bug 1265433 - Move bookmarks star into top bar of menu r?sebastian Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47631/diff/2-3/

This is (a screenshot of) what the menu looks like before sharing anything.

This is (a screenshot of) what the menu looks like after sharing once.

This is (a screenshot of) what the menu looks like after sharing 2 or more times (the most recent 2 items are listed, the left one, i.e. center icon, is the most recent share).

Comment on attachment 8743188 [details] MozReview Request: Bug 1265433 - Move bookmarks star into top bar of menu r?sebastian https://reviewboard.mozilla.org/r/47631/#review46705 LGTM. The star looks a bit "frayed" on the screenshots (It looks better on an actual device). Do we need higher quality resources here?

https://hg.mozilla.org/integration/fx-team/rev/1a5b75c9e522cf6f647701ac550d3d5f1ebfecce Bug 1265433 - Pre: remove unnecessary v11 checks r=me https://hg.mozilla.org/integration/fx-team/rev/f164661f36deef3d9e86a5a14263c4dc04613347 Bug 1265433 - Pre: remove v11 prefixes from menus r=sebastian https://hg.mozilla.org/integration/fx-team/rev/8faf7ba00b3148a01db1701e80c1c3e611c4066d Bug 1265433 - Move bookmarks star into top bar of menu r=sebastian

https://hg.mozilla.org/mozilla-central/rev/1a5b75c9e522 https://hg.mozilla.org/mozilla-central/rev/f164661f36de https://hg.mozilla.org/mozilla-central/rev/8faf7ba00b31

Verified as fixed using: Device: Nexus 6 (Android 6.0) Build: Firefox for Android 49.0a1 (2015-05-03)

https://hg.mozilla.org/releases/mozilla-aurora/rev/fae69cba7787 https://hg.mozilla.org/releases/mozilla-aurora/rev/491692205417 https://hg.mozilla.org/releases/mozilla-aurora/rev/7cca65b7ac18

Verified as fixed using: Devices: Moto X (Android 4.4), Nexus 7 (Android 5.1.1) Build: Firefox for Android 48.0a2 (2016-06-01)