This bug builds upon Bug 934029 (UX work) to add the actual button code for switch to Metro Firefox. The actual work for switching is already complete. p=1 msamuel this is another good one you can start on soon. Just copy one of the other firefox buttons as the asset until you get the ones from Bug 934029.

Here's code for checking if we are already default: > shell = Components.classes["@mozilla.org/browser/shell-service;1"]. > getService(Components.interfaces.nsIShellService); > > let isDefault = shell.isDefaultBrowser(false, false); Note the first param is false because it's not a startup check. Note the second param is false because we only want to check for http.

To show the default browser prompt if you're not the default browser do this: shell.setDefaultBrowser(claimAllTypes, false);

To show the default browser prompt if you're not the default browser and the user clicks the button, do this: > shell.setDefaultBrowser(false, false); The first false will only show the little flyout instead of opening control panel The second false is because we only want to set the default for the current user.

All of these changes for this bug should be behind: #ifdef HAVE_SHELL_SERVICE #ifdef XP_WIN #ifdef MOZ_METRO

After you launch the setDefaultBrowser you won't know if they actually selected us or not. If they did then you would launch the switch to Metro code from the other bug. Also if they already have us as default you do that initially. In preferences, we put a timer like this: http://dxr.mozilla.org/mozilla-central/source/browser/components/preferences/advanced.js#l40 Then we update the UI in preferences to not show set as default if they selected us. We should do the same here, but with some maximum timeout so we aren't running this timeout forever. (Every second with a max of let's say 10 seconds). Upgrading this from the previous p=1 to p=2. There's quite a few things to do but it's spec'ed out pretty well.

p += 1 to account for updated UX design.

* I used the icon for 'customize' for now. * All the other patches had to be rebased on the australis branch but I'm only uploading the update 'switch to metro' patch

Comment on attachment 830487 [details] [diff] [review] v1: Switch to Metro with Australis UI Review of attachment 830487 [details] [diff] [review]: ----------------------------------------------------------------- Hey Marina, would you mind getting me a UX build? Pushing your patch queue to try along with an extra changeset like this should do the trick: https://hg.mozilla.org/projects/oak/rev/3e9597e346e4 The patch looks good overall. 1 thing that I noticed that was missing is a check like this: http://dxr.mozilla.org/mozilla-central/source/browser/components/nsBrowserGlue.js#736 we want to only show the button when we're on windows 6.2 and above. Could you also do a similar additional patch for the 6.2 check for the menu item in bug 934032? You made the right call by the way by only rebasing locally for the other patches for now. There's an equal chance we'll land our stuff on m-c before Australis.

Also please ask UX for the asset if you're missing it sliced up or whatever, thanks! :D

This seems like a lot of UI realestate, has a normal toolbar button been considered? Presumably this should also be removable (as are normal toolbar buttons) for people who don't intend to use the Metro flavor of Firefox? Also, note that the panel design is changing a bit (bug 878065, see URL field for mockup).

Yes, mmaslaney and I thought about using the normal icon size in the panel. We agreed that this switching function is not a contextual command which targets the current tab or window, but a browser-level command. Clicking on the link will close Classic Firefox and switch the users to Metro mode. So we think it should be treated as the same level as "Customize" and "Exit". Since it's a browser-level command and its position is supposed be fixed, it shouldn't be customized the same way as other commands in the Customization panel. An option to turn the switch ON and OFF is probably better. I have to blame MS for not having a clear brand name for Metro. I know the term "Windows 8 style + brand" is long but that's the best way we can display this option without confusion for now.

A try build with the changes: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/msamuel@mozilla.com-296db8ec200e/try-win32/firefox-28.0a1.en-US.win32.installer.exe

Talk to Laura Forrest who recommended "Touch Friendly Mode". Also, the Icon and messaging should be left aligned, not centered. This may change, but will know more after our design critique.

Ignore the placement of customize and help.

I removed the 'add-ons' button for now (I'm just using its icon). I'm not sure what button we want 'metro mode' to replace

Attached, Firefox Touch Menu and Toolbar icon set.

Resized.

Comment on attachment 831485 [details] [diff] [review] v2: Switch to Metro with Australis UI (option 2) Review of attachment 831485 [details] [diff] [review]: ----------------------------------------------------------------- per IRC we need to explicitly hide the button pre win8

Only add switch-to-metro-button if in Windows 8.

Comment on attachment 833124 [details] [diff] [review] v2: Switch to Metro with Australis UI (option 2) Review of attachment 833124 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/customizableui/src/CustomizableUI.jsm @@ +148,5 @@ > + > +#ifdef XP_WIN > +#ifdef MOZ_METRO > + // Show switch-to-metro-button if in Windows 8. > + let isMetro = false; nit: Please call this isMetroCapable because we're not actually in metro

Includes new metro icons \o/

Landed on oak here: https://hg.mozilla.org/projects/oak/rev/8c16b0b4853d Bug 935099 will track the landing on m-c. When it lands on m-c a new comment will be added here as well with the m-c changeset. This is being done so we can still use scrumbugs efficiently. See bug 935099 for further details.

jaws: if you land your patches, please separate out the australis specific patch and place [Australis] in the commit message so it will be backed out when merging over to Holly Marina, looks like we're going to have to land the stuff in bug 924914 on Holly. Could you put the common functionality back in bug 924914, and then put the australis specific work here? Sorry I know this is a pain but it's out of our control :) Patch the patches that are for Holly only with [Holly]

I updated bug 924914 with a pre-australis patch and I believe the current patch here should be good for australis.

marina looks like this is causing a test failure here: https://tbpl.mozilla.org/?tree=Oak&rev=8c16b0b4853d Could you take a look?

So it looks like the failures were due to the fact that tests were looking for an add-ons button which was replaced with a metro mode button. This patch puts the add-ons button back where it was, except for when in win8, and it marks the two failing tests to be skipped in Windows. #ifdef doesn't seem to work in the tests so I think windows would need separate tests from these. For now, here is the try build I'm waiting on: https://tbpl.mozilla.org/?tree=Try&rev=bbd718593fe6 which hopefully should fix the test failures.

Comment on attachment 8336373 [details] [diff] [review] address-test-failures Review of attachment 8336373 [details] [diff] [review]: ----------------------------------------------------------------- I don't think there's anything specific to addon functionality, just addon id. Also we wouldn't want to disable the tests on non-win8 windows. Let's instead change the tests when XP_WIN is defined and when the os version >= 6.2 ::: browser/components/customizableui/src/CustomizableUI.jsm @@ +143,5 @@ > "history-panelmenu", > "fullscreen-button", > "find-button", > + "preferences-button", > + "add-ons-button", I think this patch should put back the addons button but leave the below off. @@ +165,1 @@ > panelPlacements.push("switch-to-metro-button"); Let's do this change in a followup bug, and flag gavin to see which button should be replaced. Please mention that bbondy thinks it may make sense to replace the full screen button with the metro one instead of the addon one. Please block the shared profile bug with that new bug.

Just putting the addons back may be enough to fix the test failure in this bug and you can handle the rest in the new bug. As long as we can grab the button from the customizations section that's good enough for this bug.

This patch removes the 'Metro Mode' button from the panel as a default button, but it can still be added as a custom button. Bug 942915 will decide where to place this button. Tests pass locally, Waiting on try here: https://tbpl.mozilla.org/?tree=Try&rev=52788555dbe1

Comment on attachment 8337847 [details] [diff] [review] address-test-failures Review of attachment 8337847 [details] [diff] [review]: ----------------------------------------------------------------- I'll land this on oak now

https://hg.mozilla.org/integration/fx-team/rev/bd024cf6af34 https://hg.mozilla.org/integration/fx-team/rev/b22896e60da9

https://hg.mozilla.org/mozilla-central/rev/bd024cf6af34 https://hg.mozilla.org/mozilla-central/rev/b22896e60da9

Verified as fixed, for iteration #19, with latest Nightly, 2013-12-01, on Win 8.1 Pro 64-bit. The 'Metro Mode' button opens Metro when clicked and it can be added as a custom button. (as shown in the attached screenshot)