924914 - Add "Relaunch in Windows 8 Touch (Metro)" feature to Desktop Firefox

Status: RESOLVED FIXED

(Firefox :: General, defect, P2)

Description

[Brian R. Bondy [:bbondy]]

This is dependent on UX, but my suggestion is to add a toolbar button next to the downloads one for switch to Metro. Or use the top left menu bar pre-australis and add a menu item.

To just get the core of the feature working, I'd suggest adding it as a menu item for now until UX gives more clarification.

We should make a call like this in the event handler:
http://dxr.mozilla.org/mozilla-central/source/widget/windows/winrt/MetroAppShell.cpp#l200

Specifying the new eRestartTouchEnvironment flag.

We also need to add support for handling that flag inside toolkit code.  The handling of that flag should simply do this:
http://dxr.mozilla.org/mozilla-central/source/widget/windows/winrt/MetroAppShell.cpp#l200

p=5

Comment 1

[Marina Samuel [:emtwo]]

Looking at the v1 backlog, I found bug 915375 which I think sounds about the same as this one. Is this perhaps something we want regardless of whether we share profiles?

Comment 2

[Brian R. Bondy [:bbondy]]

yep it's pretty much the same, but this bug is for restarting and switching, and the other one would probably just open a second instance, but in the metro environment. I think we should wait for a decision to figure out which one we should close.

Comment 3

[Brian R. Bondy [:bbondy]]

Sorry my last comment was incorrect. This bug just had the wrong title, it did have the correct description though.  Now the title is also fixed.

Comment 5

[Brian R. Bondy [:bbondy]]

The current plan is to add a menu item in the orange firefox menu on the top left.
Are you OK with that? We also need a separate task for the UX branch since that menu goes away there.

Comment 6

[Marina Samuel [:emtwo]]

Attachment: v1: Switch to Metro

Comment 7

[Brian R. Bondy [:bbondy]]

Comment on attachment 824712 [details] [diff] [review]
v1: Switch to Metro

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

Looks good at a first pass, I'd like to do another pass at it after the switch to desktop bug is done though.
I want to make sure that the profile is completely done being used at the point you are launching the metro browser, I think it is.

Comment 8

[Brian R. Bondy [:bbondy]]

Comment on attachment 824712 [details] [diff] [review]
v1: Switch to Metro

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

Couple nits.

::: browser/base/content/browser.js
@@ +2519,5 @@
>  }
>  
> +function SwitchToMetro() {
> +  let appStartup = Components.classes["@mozilla.org/toolkit/app-startup;1"].
> +                    getService(Components.interfaces.nsIAppStartup);

nit: just align this second line by 2 spaces only.

::: browser/locales/en-US/chrome/browser/browser.dtd
@@ +411,5 @@
>  <!ENTITY bookmarkThisFrameCmd.label      "Bookmark This Frame">
>  <!ENTITY bookmarkThisFrameCmd.accesskey  "m">
>  <!ENTITY emailPageCmd.label           "Email Link…">
>  <!ENTITY emailPageCmd.accesskey       "E">
> +<!ENTITY switchToMetroCmd.label       "Switch to Metro">

nit: Relaunch in Windows 8 style &brandShortName

::: toolkit/components/startup/nsAppStartup.cpp
@@ +273,5 @@
>  
> +  nsresult retval = NS_OK;
> +  if (mRestart)
> +    retval = NS_SUCCESS_RESTART_APP;
> +  if (mRestartTouchEnvironment)

nit: Check mRestartTouchEnvironment first an then have else if (mRestart)

Comment 9

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

(In reply to Brian R. Bondy [:bbondy] from comment #5)
> The current plan is to add a menu item in the orange firefox menu on the top
> left.
> Are you OK with that? We also need a separate task for the UX branch since
> that menu goes away there.

Hi Brian,

I just talked to designer who work on Australis, for the new interface, it's likely that this option will be hidden in the "Customize" panel. We will need an icon for the action.

And I've been thinking about use cases for opening links from Classic to Metro. And here is a question: based on your current plan, if a users have multiple tabs open in Classic FX, will switching to Metro FX load all his tabs, or only the current one in the foreground?

Comment 10

[Brian R. Bondy [:bbondy]]

(In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #9)
> (In reply to Brian R. Bondy [:bbondy] from comment #5)
> > The current plan is to add a menu item in the orange firefox menu on the top
> > left.
> > Are you OK with that? We also need a separate task for the UX branch since
> > that menu goes away there.
> 
> Hi Brian,
> 
> I just talked to designer who work on Australis, for the new interface, it's
> likely that this option will be hidden in the "Customize" panel. We will
> need an icon for the action.

I'd like for it to be present only on windows 8, and it would be better if it was top level on the desktop browser itself (i.e. not in the customize menu). I'd also like that if the browser is not the default, when you pressed it, it would ask for default status first.  This would give us much better discoverability.

Thoughts?

> 
> And I've been thinking about use cases for opening links from Classic to
> Metro. And here is a question: based on your current plan, if a users have
> multiple tabs open in Classic FX, will switching to Metro FX load all his
> tabs, or only the current one in the foreground?

yes all of the tabs, all of the login status will also be preserved.  Also even if you have data filled out in forms that will also be preserved.

Comment 11

[Brian R. Bondy [:bbondy]]

BTW Marina, the ux branch stuff we'll do in another bug, we're just discussing it here though :)

Comment 12

[Brian R. Bondy [:bbondy]]

Also Yuan, whether we make it top level or not, please go ahead with an icon because we'll need it in either case.

Comment 13

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

(In reply to Brian R. Bondy [:bbondy] from comment #10)
> (In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #9)
> > (In reply to Brian R. Bondy [:bbondy] from comment #5)
> > > The current plan is to add a menu item in the orange firefox menu on the top
> > > left.
> > > Are you OK with that? We also need a separate task for the UX branch since
> > > that menu goes away there.
> > 
> > Hi Brian,
> > 
> > I just talked to designer who work on Australis, for the new interface, it's
> > likely that this option will be hidden in the "Customize" panel. We will
> > need an icon for the action.
> 
> I'd like for it to be present only on windows 8, and it would be better if
> it was top level on the desktop browser itself (i.e. not in the customize
> menu). I'd also like that if the browser is not the default, when you
> pressed it, it would ask for default status first.  This would give us much
> better discoverability.
> 
> Thoughts?

That's the ideal solution. I will keep pushing for that, especially if a Windows 8 version menu panel is possible.
The default choice makes sense.
 

> 
> > 
> > And I've been thinking about use cases for opening links from Classic to
> > Metro. And here is a question: based on your current plan, if a users have
> > multiple tabs open in Classic FX, will switching to Metro FX load all his
> > tabs, or only the current one in the foreground?
> 
> yes all of the tabs, all of the login status will also be preserved.  Also
> even if you have data filled out in forms that will also be preserved.

One more question: How about multiple windows? Open only tabs from the current window?


A hypothesis I have is that the No.1 scenario for switching from Class to Metro is context switching. For example, a users want to keep reading articles he saved on desktop FX when he is on the go (without keyboard attached). 

For people who have lots of tabs open on desktop, opening everything on Metro could be an over-kill.

I am thinking for this use case, it might be better to let users pick what they would like to open in Metro. Show a similar in-content page as "Restore tabs" page on FX desktop, where users can select what to open in Metro(they can also choose to open all). This panel only shows when there are lots of tabs open(over 15?)

Thoughts?

Comment 14

[Brian R. Bondy [:bbondy]]

(In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #13)
> (In reply to Brian R. Bondy [:bbondy] from comment #10)
> > (In reply to Yuan Wang(:Yuan) – Firefox UX Team from comment #9)
> One more question: How about multiple windows? Open only tabs from the
> current window?

It will use session restore so I think it would treat each as its own tab.

> A hypothesis I have is that the No.1 scenario for switching from Class to
> Metro is context switching. For example, a users want to keep reading
> articles he saved on desktop FX when he is on the go (without keyboard
> attached). 

Agreed.

> For people who have lots of tabs open on desktop, opening everything on
> Metro could be an over-kill.

I think we should let session restore handle this. I think when there's too many tabs it will show them as a list instead and allow you to unselect them and then restore.

> I am thinking for this use case, it might be better to let users pick what
> they would like to open in Metro. Show a similar in-content page as "Restore
> tabs" page on FX desktop, where users can select what to open in Metro(they
> can also choose to open all). This panel only shows when there are lots of
> tabs open(over 15?)
> Thoughts?

I'm not opposed to exploring this, but let's not implement it for v1. Let's look at improving that part of things for v2.

For the image for the ux branch customization panel, do you want me to post another bug? Maybe you can fight for it to be auto-promoted by saying it is only for one OS and only for win8 and 8.1. So it is only for a small percentage of the users overall.

Comment 15

[Marina Samuel [:emtwo]]

Attachment: v2: Switch to Metro

Comment 16

[Brian R. Bondy [:bbondy]]

Comment on attachment 824712 [details] [diff] [review]
v1: Switch to Metro

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

::: toolkit/xre/nsAppRunner.cpp
@@ +124,5 @@
>  #include "nsAppDirectoryServiceDefs.h"
>  #include "nsXULAppAPI.h"
>  #include "nsXREDirProvider.h"
>  #include "nsToolkitCompsCID.h"
> +#include "updatehelper.h"

This needs to be inside of an ifdef:
#if defined(XP_WIN)
#include "updatehelper.h"
#endif

It's only exported on Windows.

@@ +4049,5 @@
>  #endif
>  
>    // Check for an application initiated restart.  This is one that
>    // corresponds to nsIAppStartup.quit(eRestart)
> +  if (rv == NS_SUCCESS_RESTART_APP || rv == NS_SUCCESS_RESTART_METRO_APP) {

It's ok to leave this out of the ifdef as long as you keep it defined everywhere.

@@ +4054,3 @@
>      appInitiatedRestart = true;
> +    if (rv == NS_SUCCESS_RESTART_METRO_APP) {
> +      LaunchDefaultMetroBrowser();

ditto: This should be inside an:
#if defined(MOZ_METRO) && defined(XP_WIN)
if (rv == NS_SUCCESS_RESTART_METRO_APP) {
		LaunchDefaultMetroBrowser();
}
#endif

Comment 17

[Marina Samuel [:emtwo]]

Comment on attachment 825975 [details] [diff] [review]
v2: Switch to Metro

Oops, I missed the new comments before uploading this

Comment 18

[Marina Samuel [:emtwo]]

Attachment: Patch 1 - Switch to Metro. rev3.

Comment 19

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/projects/oak/rev/deb77824578d

Comment 20

[Marina Samuel [:emtwo]]

Attachment: relaunch_in_windows8.png

A separate bug will be posted or the UX branch once an icon is received.

Comment 21

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Thanks, Marina. It looks good to me.
I will talk to Firefox UX team on Tuesday to finalize the copy. Will keep you posted.

Comment 22

[Brian R. Bondy [:bbondy]]

Comment on attachment 825991 [details] [diff] [review]
Patch 1 - Switch to Metro. rev3.

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

::: toolkit/xre/nsAppRunner.cpp
@@ +125,5 @@
>  #include "nsXULAppAPI.h"
>  #include "nsXREDirProvider.h"
>  #include "nsToolkitCompsCID.h"
>  
> +#if defined(XP_WIN)

Looks like we need:
#if defined(XP_WIN) && defined(MOZ_METRO) here
https://tbpl.mozilla.org/?tree=Oak&rev=a384228760da
B2G win builds are failing otherwise.
I'll fix on oak, could you upload a new patch for here? Thanks.

Comment 23

[Brian R. Bondy [:bbondy]]

Marina ignore comment 22, I'll post a patch here.

Landed on oak here:
https://hg.mozilla.org/projects/oak/rev/deb77824578d
https://hg.mozilla.org/projects/oak/rev/2569b7424bbd

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.

Comment 24

[Brian R. Bondy [:bbondy]]

Attachment: Patch 2 - Fix for include guard

The main patch had a build problem on oak with b2g windows builds. This fixes that build problem.

Comment 25

[Jim Mathies [:jimm]]

Comment on attachment 825991 [details] [diff] [review]
Patch 1 - Switch to Metro. rev3.

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

::: browser/locales/en-US/chrome/browser/browser.dtd
@@ +411,5 @@
>  <!ENTITY bookmarkThisFrameCmd.label      "Bookmark This Frame">
>  <!ENTITY bookmarkThisFrameCmd.accesskey  "m">
>  <!ENTITY emailPageCmd.label           "Email Link…">
>  <!ENTITY emailPageCmd.accesskey       "E">
> +<!ENTITY switchToMetroCmd.label       "Relaunch in Windows 8 style &brandShortName;">

drive by comment - "Relaunch in" seems odd to me, maybe use "Switch to"?

Comment 26

[Manuela Muntean [Away]]

Verified as fixed for iteration #18, on Win 8 64-bit, with the latest oak build (build ID: 20131114040200).

When in Desktop mode and pressing the "Relaunch in Windows 8 style Nightly" button, the Metro mode is launched with the exact tabs opened as in the Desktop mode.

Also, when in Metro mode and pressing the "Relaunch in Desktop" button, the Desktop mode is launched with the exact tabs opened as in the Metro mode.

Comment 27

[Marina Samuel [:emtwo]]

Comment on attachment 826183 [details]
relaunch_in_windows8.png

This is obsolute since we're using the UX design decided on in bug 934029

Comment 28

[Brian R. Bondy [:bbondy]]

Comment on attachment 825991 [details] [diff] [review]
Patch 1 - Switch to Metro. rev3.

emtwo mentioned all of this is included in the Australis path, so obsoleting this.

Comment 29

[Brian R. Bondy [:bbondy]]

Comment on attachment 827528 [details] [diff] [review]
Patch 2 - Fix for include guard

Included in australis bug.

Comment 30

[Brian R. Bondy [:bbondy]]

Patches included in bug 934032, nothing in this bug will land on m-c because astralis landed before us.

Comment 31

[Marina Samuel [:emtwo]]

Attachment: v4: Switch to Metro

Patch for pre-australis on Holly.

Comment 32

[Marina Samuel [:emtwo]]

Re-opening to land patch in Holly

Comment 33

[Brian R. Bondy [:bbondy]]

Comment on attachment 8334744 [details] [diff] [review]
v4: Switch to Metro

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

::: toolkit/xre/nsAppRunner.cpp
@@ +4057,5 @@
>      appInitiatedRestart = true;
>  
> +#if defined(MOZ_METRO) && defined(XP_WIN)
> +    if (rv == NS_SUCCESS_RESTART_METRO_APP) {
> +      LaunchDefaultMetroBrowser();

Is this code that will already be included in the UX branch patch?
I think so, we need to separate out only the parts for Holly that won't be merged in from m-c.

Ditto other changes in this file.

Comment 34

[Brian R. Bondy [:bbondy]]

Moving to fixed since it's already on oak, but we still need to split up the previous patch a per the last comment.

Comment 35

[Brian R. Bondy [:bbondy]]

This patch, or some form close to it will be landing on Holly only. It adds a "Relaunch in Metro" feature to Desktop Firefox's top left menu.  It has a l10n string in it.  Is there anything special we need to do to make sure it will get seen and localized before Holly goes to aurora?  It's possible there are other bugs from other people that have similar UI string changes that apply to Holly only, I just wanted to make sure this edge case is covered.

Comment 36

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Brian R. Bondy [:bbondy] from comment #35)
> This patch, or some form close to it will be landing on Holly only. It adds
> a "Relaunch in Metro" feature to Desktop Firefox's top left menu.  It has a
> l10n string in it.  Is there anything special we need to do to make sure it
> will get seen and localized before Holly goes to aurora?  It's possible
> there are other bugs from other people that have similar UI string changes
> that apply to Holly only, I just wanted to make sure this edge case is
> covered.

Please land the strings that you need on m-c with a separate patch and that patch will be merged over to Holly. The rest of your patch can land directly on Holly, but the strings need to be introduced on m-c.

Comment 37

[Marina Samuel [:emtwo]]

Attachment: [Holly only] v5: Switch to Metro

This patch does not include the string or common code in mc

Comment 38

[Marina Samuel [:emtwo]]

Attachment: Added string

patch for mc with just the added string

Comment 39

[Manuela Muntean [Away]]

Attachment: screenshot.png

For iteration #19, I've tried testing this with the latest Holly build installer from ftp://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/holly-win32/1385580442/ , build ID: 20131127074557, on Win 8 64bit, but I couldn't find the "Relaunch in Windows 8 style Nightly" button, as shown in the attached screenshot.

I assume this hasn't been landed on Holly yet.

Comment 40

[Brian R. Bondy [:bbondy]]

This work is only on oak right now, it'll be moving soon though.

Comment 41

[Manuela Muntean [Away]]

(In reply to Brian R. Bondy [:bbondy] from comment #40)
> This work is only on oak right now, it'll be moving soon though.

I didn't find the "Relaunch in Windows 8 style Nightly" button, with the latest oak build, from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-oak/ (the oak build comes with Australis)

Comment 42

[Brian R. Bondy [:bbondy]]

It'll be in the customization panel for now

Comment 43

[Manuela Muntean [Away]]

Attachment: screenshot_Customize.png

Indeed, I could see the button (as shown in the screenshot), after customizing a little. Thanks!

Comment 44

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/integration/fx-team/rev/514b65ab771d

Comment 45

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/514b65ab771d

Comment 46

[Gary [:streetwolf52]]

This patch lightens the icons that on the Panel Menu that are placed on the various toolbars such as the navigation bar, bookmarks bar, etc.

Regression
----------

Good:  20131128150519
http://hg.mozilla.org/integration/fx-team/rev/5a6373ab5fe6

Bad: 20131128171419
http://hg.mozilla.org/integration/fx-team/rev/514b65ab771d


Mozilla/5.0 (Windows NT 6.3; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0

Comment 47

[Gary [:streetwolf52]]

Attachment: Light colored icons after this patch

Comment 48

[Gary [:streetwolf52]]

Attachment: Norml icons before this patch

Comment 49

[Gary [:streetwolf52]]

Also lightens the Back/Forward buttons.

Comment 50

[Brian R. Bondy [:bbondy]]

Not this bug, but bug 934032.
I'll post this info in another new bug and mark it as block bug 934032.

Comment 51

[Gary [:streetwolf52]]

(In reply to Brian R. Bondy [:bbondy] from comment #50)
> Not this bug, but bug 934032.
> I'll post this info in another new bug and mark it as block bug 934032.

I thought that the problem didn't seem to fit this patch even though my regression testing pointed here. Where did I go wrong in regression testing so I don't do it again?

Comment 52

[Brian R. Bondy [:bbondy]]

there were 14 patches landed in 1 push. The bug I pointed to was in the same push as this bug was.

Comment 53

[Gary [:streetwolf52]]

I see now. Shouldn't happen again.

Comment 54

[Brian R. Bondy [:bbondy]]

No worries, thanks for finding the regression range! :)

Comment 55

[Brian R. Bondy [:bbondy]]

https://hg.mozilla.org/projects/holly/rev/73388ef135b3

Comment 56

[Yuan Wang(:Yuan) – Mobile Firefox Design Lead]

Since the new name is finalized, we should use "Relaunch in *Brand* for Windows 8 Touch" for non-Australis desktop UI.

Comment 57

[Brian R. Bondy [:bbondy]]

Marina, could you see to it that the strings are updated on Holly and if any are left on m-c?

Comment 58

[Manuela Muntean [Away]]

Verified as fixed, for iteration #19, on Win 8 64-bit, with latest Holly build 2013-12-06.

In desktop mode, I can see the "Relaunch in Windows 8 style Holly" menu item, that launches Metro mode.

Comment 60

[Matt Brubeck (:mbrubeck)]

Adding this to the query on https://wiki.mozilla.org/Features/Release_Tracking