STR: 1) Open Firefox in a new profile on Windows 8, and enable the menubar (Firefox button > Options > Menu Bar) 2) Attempt to enter Metro mode from the newly exposed menu bar ER: There should be a menuitem in the menubar to allow the user to go to Metro mode. AR: There is no such menuitem, at least, not that I can see. Filed on behalf of matej, who I am Cc'ing.

(In reply to Marina Samuel [:emtwo] from comment #1) > Created attachment 8381801 [details] [diff] [review] > v1: Add "Relaunch in Windows 8 Touch" button to the menu bar in beta+ Is this language deliberately different? In the Firefox menu, it says "Relaunch in Firefox for Windows 8 Touch." Unless there's a good reason, I think we should be consistent. I also wonder if it needs to say "Beta" somewhere. In Aurora it says "Relaunch in Aurora for Windows 8 Touch." Should this, perhaps, say "Relaunch in Firefox Beta for Windows 8 Touch" (of course, then it would have to be changed in both places)? Thanks.

(In reply to Matej Novak [:matej] from comment #2) > (In reply to Marina Samuel [:emtwo] from comment #1) > > Created attachment 8381801 [details] [diff] [review] > > v1: Add "Relaunch in Windows 8 Touch" button to the menu bar in beta+ > > Is this language deliberately different? In the Firefox menu, it says > "Relaunch in Firefox for Windows 8 Touch." Unless there's a good reason, I > think we should be consistent. We are - I suspect Marina was just giving a precis of the change. The string that she's using is the exact same one being used in the Firefox menu - specifically: "Relaunch in &brandShortName; for Windows 8 Touch" > > I also wonder if it needs to say "Beta" somewhere. In Aurora it says > "Relaunch in Aurora for Windows 8 Touch." > > Should this, perhaps, say "Relaunch in Firefox Beta for Windows 8 Touch" (of > course, then it would have to be changed in both places)? > At this point, Beta branding is essentially identical to Release. The update channel is different, but appearance-wise, they're not distinguishable. I'm not sure if we want to switch Beta to be more distinguishable, but even if we did, changing strings on the Beta channel is a no-can-do localization-wise. I suspect that's the case even for branding.

Comment on attachment 8381801 [details] [diff] [review] v1: Add "Relaunch in Windows 8 Touch" button to the menu bar in beta+ Review of attachment 8381801 [details] [diff] [review]: ----------------------------------------------------------------- This is fine - just please change the ID of the menuitem so that we don't collide. Thanks Marina! ::: browser/base/content/browser-menubar.inc @@ +40,5 @@ > accesskey="&openFileCmd.accesskey;"/> > +#ifdef HAVE_SHELL_SERVICE > +#ifdef XP_WIN > +#ifdef MOZ_METRO > + <menuitem id="switch-to-metro" The id being used for the appmenu button menuitem also has the ID "switch-to-metro" - ID collision is not a thing we should do. I suggest calling this something like "menu_switchToMetro" instead.

Thanks for the extra context. Looks god to me.

Made id change and carrying over r+ from mconley [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 924914 User impact if declined: Users who are familiar with and often use the menubar will not easily find a "switch to metro mode" option Testing completed (on m-c, etc.): tested locally on beta branch Risk to taking this patch (and alternatives if risky): low risk, adding a button with already existing functionality to the menubar String or IDL/UUID changes made by this patch: none

Comment on attachment 8382383 [details] [diff] [review] v2: Add "Relaunch in Windows 8 Touch" button to the menu bar in beta+ Going ahead and requesting this for Aurora as well, to make sure it doesn't slip through the cracks.

Comment on attachment 8382383 [details] [diff] [review] v2: Add "Relaunch in Windows 8 Touch" button to the menu bar in beta+ So I just talked to matej about this, and because (at least for now) Australis is riding Aurora, we might not want to land this patch there. The reasoning being that we're only putting this item in the menubar because on Windows, if the menubar is enabled, the appmenu button is gone, and there's no way to get to Metro mode. With Australis, the menubar being enabled has no bearing on the availability of the Metro toggle in the menu panel. So, withdrawing my request for Aurora unless there's any objections here. Marina - you will, however, probably want to land this change on Holly, in the event that Australis does not ride the 29 train.

Marina, any reason why this didn't land in m-c before? thanks

Comment on attachment 8382383 [details] [diff] [review] v2: Add "Relaunch in Windows 8 Touch" button to the menu bar in beta+ OK - approving this for the FF28 beta and leaving the rest tracked so we can have this on our radar just in case we don't put Australis to Beta. To be clear, nothing further is needed if Australis does go forward?

(In reply to Lukas Blakk [:lsblakk] from comment #10) > Comment on attachment 8382383 [details] [diff] [review] > v2: Add "Relaunch in Windows 8 Touch" button to the menu bar in beta+ > > OK - approving this for the FF28 beta and leaving the rest tracked so we can > have this on our radar just in case we don't put Australis to Beta. To be > clear, nothing further is needed if Australis does go forward? That's correct - and is also why this didn't land on Nightly (this patch is not needed for builds with Australis).

So I just got off the horn with shorlander in UX, and I think we're reversing this decision here. We're going to go for landing this on post-Australis builds as well. Sorry for the thrashing, y'all.

Comment on attachment 8382383 [details] [diff] [review] v2: Add "Relaunch in Windows 8 Touch" button to the menu bar in beta+ [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 924914 User impact if declined: Users who are familiar with and often use the menubar will not easily find a "switch to metro mode" option Testing completed (on m-c, etc.): tested locally on beta branch Risk to taking this patch (and alternatives if risky): low risk, adding a button with already existing functionality to the menubar String or IDL/UUID changes made by this patch: none

This was landed on fx-team, though the bug wasn't marked as such. https://hg.mozilla.org/integration/fx-team/rev/2542878529f8

Does this have or need in-testsuite tests?

Landed in holly: https://hg.mozilla.org/projects/holly/rev/7b1567b25dc9

For testing and verification. Reopen if any defects found.

Went through the verification process using following Nightly build: - http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-03-03-03-02-01-mozilla-central/ Before verification, reproduced the original issue using the following build: - http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-02-20-03-02-02-mozilla-central/ - Ensured that the position is consistant throughout all the channels (Nightly, Aurora & BETA) - Ensured that the wording is consistant throughout all the channels (Nightly, Aurora & BETA) - Ensured that selecting the menu item correctly launches fxmetro - Ensured that you can switch back and forth between both fxmetro and fxdesktop - Went through all of the above test cases in several different variations of snapped view - Went through all of the above test cases using both the X1 Carbon and the Surface Pro 2