No option to Relaunch in Metro mode from old-style menubar

VERIFIED FIXED in Firefox 28

Status

defect
P1
normal
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: mconley, Assigned: emtwo)

Tracking

unspecified
Firefox 30
x86
Windows 8
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox27 unaffected, firefox28+ verified, firefox29+ verified, firefox30+ verified, b2g-v1.3 fixed)

Details

(Whiteboard: p=1 s=it-30c-29a-28b.2 r=ff28)

Attachments

(1 attachment, 1 obsolete attachment)

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.
Blocks: metrobacklog
Whiteboard: [triage]
(Assignee)

Updated

5 years ago
Assignee: nobody → msamuel
(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.
(Reporter)

Comment 3

5 years ago
(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.
(Reporter)

Comment 4

5 years ago
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.
Attachment #8381801 - Flags: review?(mconley) → review+
Thanks for the extra context. Looks god to me.
(Assignee)

Comment 6

5 years ago
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
Attachment #8381801 - Attachment is obsolete: true
Attachment #8382383 - Flags: review+
Attachment #8382383 - Flags: approval-mozilla-beta?
(Reporter)

Comment 7

5 years ago
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.
Attachment #8382383 - Flags: approval-mozilla-aurora?
(Reporter)

Comment 8

5 years ago
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.
Attachment #8382383 - Flags: approval-mozilla-aurora?
Marina, any reason why this didn't land in m-c before? thanks
Flags: needinfo?(msamuel)
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?
Attachment #8382383 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(Reporter)

Comment 12

5 years ago
(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).
Flags: needinfo?(msamuel)
(Reporter)

Comment 13

5 years ago
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.
Keywords: checkin-needed
(Reporter)

Comment 14

5 years ago
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
Attachment #8382383 - Flags: approval-mozilla-aurora?
Status: NEW → ASSIGNED
Priority: -- → P1
QA Contact: kamiljoz
Whiteboard: [triage] → p=1 s=it-30c-29a-28b.2 r=ff28
This was landed on fx-team, though the bug wasn't marked as such.
https://hg.mozilla.org/integration/fx-team/rev/2542878529f8
Keywords: checkin-needed
Whiteboard: p=1 s=it-30c-29a-28b.2 r=ff28 → p=1 s=it-30c-29a-28b.2 r=ff28[fixed-in-fx-team]
Does this have or need in-testsuite tests?
Flags: in-testsuite?
https://hg.mozilla.org/mozilla-central/rev/2542878529f8
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: p=1 s=it-30c-29a-28b.2 r=ff28[fixed-in-fx-team] → p=1 s=it-30c-29a-28b.2 r=ff28
Target Milestone: --- → Firefox 30
Attachment #8382383 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
For testing and verification.  Reopen if any defects found.
Flags: needinfo?(kamiljoz)
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
Status: RESOLVED → VERIFIED
Flags: needinfo?(kamiljoz)
You need to log in before you can comment on or make changes to this bug.