Closed Bug 1164675 Opened 5 years ago Closed 4 years ago

[Tablet mode] Firefox displays useless titlebar command buttons (preview build 10074)

Categories

(Firefox :: Theme, defect, P3)

Unspecified
Windows 10
defect

Tracking

()

VERIFIED FIXED
Firefox 44
Tracking Status
firefox41 --- affected
firefox44 --- verified

People

(Reporter: jimm, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

STR:

1) open firefox in tablet mode, or open firefox and switch to tabelt mode

result: in the titlebar we have minimize, maximize, and close buttons.

2) try to click any of these buttons

result: nothing happens

expected: like other apps, these buttons should not be displayed in tablet mode.
Someone running a different build told me they had working close and minimize, so this might be an os bug as well.

FWIW Spartan doesn't display any of these buttons since the normal tablet mode gestures work. I think we should follow suit.
Summary: [Tablet mode] Firefox displays useless titlebar command buttons → [Tablet mode] Firefox displays useless titlebar command buttons (preview build 10074)
This is an issue with all Win32 apps. I'm sure MS is going to fix this for release.
(In reply to Tim Nguyen [:ntim] (limited availability) from comment #2)
> This is an issue with all Win32 apps. I'm sure MS is going to fix this for
> release.

We actually draw our own command buttons via xul thanks to draw in titlebar so I'm pretty sure this is going to have to happen in fx front end layout markup / script.
Priority: -- → P2
(In reply to Jim Mathies [:jimm] from comment #1)
> Someone running a different build told me they had working close and
> minimize, so this might be an os bug as well.
> 
> FWIW Spartan doesn't display any of these buttons since the normal tablet
> mode gestures work. I think we should follow suit.

Can you elaborate? IE seems to keep these, for instance, as does Windows Explorer... I, for one, have no idea what the "normal tablet mode gestures" are. I would also note that this mode is accessible without a touchscreen interface, and those users might not be able to use those gestures...
Flags: needinfo?(jmathies)
(In reply to :Gijs Kruitbosch from comment #4)
> (In reply to Jim Mathies [:jimm] from comment #1)
> > Someone running a different build told me they had working close and
> > minimize, so this might be an os bug as well.
> > 
> > FWIW Spartan doesn't display any of these buttons since the normal tablet
> > mode gestures work. I think we should follow suit.
> 
> Can you elaborate? IE seems to keep these, for instance, as does Windows
> Explorer... I, for one, have no idea what the "normal tablet mode gestures"
> are. I would also note that this mode is accessible without a touchscreen
> interface, and those users might not be able to use those gestures...

Only the close button should be shown in tablet mode. The maximize and minimize buttons have no effect in tablet mode (in other win32 apps too). New universal apps only keep the close button in tablet mode (that's what I see on non-touch screens at least).

The tablet mode gesture he's referring to may be the gesture to close the app (drag from the top to the bottom of the screen).
(In reply to Tim Nguyen [:ntim] from comment #5)
> (In reply to :Gijs Kruitbosch from comment #4)
> > (In reply to Jim Mathies [:jimm] from comment #1)
> > > Someone running a different build told me they had working close and
> > > minimize, so this might be an os bug as well.
> > > 
> > > FWIW Spartan doesn't display any of these buttons since the normal tablet
> > > mode gestures work. I think we should follow suit.
> > 
> > Can you elaborate? IE seems to keep these, for instance, as does Windows
> > Explorer... I, for one, have no idea what the "normal tablet mode gestures"
> > are. I would also note that this mode is accessible without a touchscreen
> > interface, and those users might not be able to use those gestures...
> 
> Only the close button should be shown in tablet mode. The maximize and
> minimize buttons have no effect in tablet mode (in other win32 apps too).
> New universal apps only keep the close button in tablet mode (that's what I
> see on non-touch screens at least).

My point is that on the newest Win10 build I see all 3 buttons for IE, Windows and Notepad; why should we behave differently? What's the rationale?
(In reply to :Gijs Kruitbosch from comment #6)
> (In reply to Tim Nguyen [:ntim] from comment #5)
> > (In reply to :Gijs Kruitbosch from comment #4)
> > > (In reply to Jim Mathies [:jimm] from comment #1)
> > > > Someone running a different build told me they had working close and
> > > > minimize, so this might be an os bug as well.
> > > > 
> > > > FWIW Spartan doesn't display any of these buttons since the normal tablet
> > > > mode gestures work. I think we should follow suit.
> > > 
> > > Can you elaborate? IE seems to keep these, for instance, as does Windows
> > > Explorer... I, for one, have no idea what the "normal tablet mode gestures"
> > > are. I would also note that this mode is accessible without a touchscreen
> > > interface, and those users might not be able to use those gestures...
> > 
> > Only the close button should be shown in tablet mode. The maximize and
> > minimize buttons have no effect in tablet mode (in other win32 apps too).
> > New universal apps only keep the close button in tablet mode (that's what I
> > see on non-touch screens at least).
> 
> My point is that on the newest Win10 build I see all 3 buttons for IE,
> Windows and Notepad; why should we behave differently? What's the rationale?

IE is a legacy desktop app now, what does Edge display?
Flags: needinfo?(jmathies)
Flags: needinfo?(jmathies)
(In reply to :Gijs Kruitbosch from comment #6)
> My point is that on the newest Win10 build I see all 3 buttons for IE,
> Windows and Notepad; why should we behave differently? What's the rationale?
Modern universal apps (including Microsoft Edge) only shows the close button. Other Win32 apps should also just show the close button (because the other 2 have no effect). That's definitively a bug on MS side. Although, for Firefox, Jim mentioned in comment 3 that we draw our own window controls, so we should probably do this on our side.
(In reply to Jim Mathies [:jimm] from comment #7)
> IE is a legacy desktop app now, what does Edge display?

It displays nothing, and displays an [x] only when you mess with the top of the screen with the mouse, which causes the x to slide in. It animates out again (seems to be flipping on its way, or something?) when you move focus elsewhere.

(NB: I'm using win10 on a VM, with no touch support)

So there's a couple of notes here:

1) xul/css/js doesn't know if we're in tablet mode or not.
2) I looked for APIs for this on MSDN yesterday night and could not find anything (but I find it very hard to find *anything* on MSDN so maybe I just wasn't looking in the right places?)
3) the titlebar buttons are rendered by core code / windows using -moz-appearance: -moz-win-titlebar-*; the style and position of the button are "wrong" for tablet mode. That likely needs native code changes as well...
(In reply to :Gijs Kruitbosch from comment #9)
> (In reply to Jim Mathies [:jimm] from comment #7)
> > IE is a legacy desktop app now, what does Edge display?
> 
> It displays nothing, and displays an [x] only when you mess with the top of
> the screen with the mouse, which causes the x to slide in. It animates out
> again (seems to be flipping on its way, or something?) when you move focus
> elsewhere.

They probably get all of this for free by being a universal app. :/

> So there's a couple of notes here:
> 
> 1) xul/css/js doesn't know if we're in tablet mode or not.
> 2) I looked for APIs for this on MSDN yesterday night and could not find
> anything (but I find it very hard to find *anything* on MSDN so maybe I just
> wasn't looking in the right places?)

Lets file a bug on this, we'll need to find it and tie it into some sort of css related query for frontend like we do with things like compositor support.

> 3) the titlebar buttons are rendered by core code / windows using
> -moz-appearance: -moz-win-titlebar-*; the style and position of the button
> are "wrong" for tablet mode. That likely needs native code changes as well...

Some of this positioning is handled in C++ down in nsNativeThemeWin.cpp.

http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsNativeThemeWin.cpp#1636
http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsNativeThemeWin.cpp#2094
Flags: needinfo?(jmathies)
Depends on: 1170522
One thing worth noting, on tablets like the surface pro windows 10 defaults to tablet mode on startup even if I've flipped it to desktop mode prior to restarting.
No longer blocks: windows-10
Duplicate of this bug: 1181547
(In reply to Tim Nguyen [:ntim] from comment #2)
> This is an issue with all Win32 apps. I'm sure MS is going to fix this for
> release.

Ah, when we were young and optimistic.

More seriously, 10240 is supposed to be RTM and this is unfixed, even on, say, Windows Explorer. Woo.

Good news though, we actually do what comment #3 said now, so we can just hide the buttons we don't want (well, for the main window...).
Depends on: 1173725
Placing this in the generic P3 tablet-mode bucket.
Priority: P2 → P3
Bug 1164675 - hide min/max/restore buttons in the main window in windows 10 tablet mode, r?jaws
Attachment #8676234 - Flags: review?(jaws)
I decided that this was going to be useful for bug 1216227 anyway, so I just fixed it.

Note that this only fixes the main Firefox windows. Everything else is up to MS.

The infrastructure this adds could also be used to fix bug 1165319. I chose not to do that because the code I just wrote is pretty much "easy win" whereas getting the swipe stuff implemented is going to be (more) difficult, and I don't know that it's worth it (we need bug 1216227 to get a more accurate picture of that).
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Blocks: 1216227
Comment on attachment 8676234 [details]
MozReview Request: Bug 1164675 - hide min/max/restore buttons in the main window in windows 10 tablet mode, r?jaws

https://reviewboard.mozilla.org/r/22635/#review20165

::: browser/base/content/browser.css:295
(Diff revision 1)
> +#main-window[tabletmode="true"] #titlebar-min,
> +#main-window[tabletmode="true"] #titlebar-max {

Please change the attribute selector to just [tabletmode] which matches the selectors below and because we don't use values other than "true" for the tabletmode attribute.

::: browser/base/content/browser.js:5417
(Diff revision 1)
> +    if (AppConstants.isPlatformAndVersionAtLeast("win", "10.0")) {

Four out of the five instances that check for Win10 use "10" and not "10.0". We should standardize on one of them. Aiui, there is no difference between "10" and "10.0".
Attachment #8676234 - Flags: review?(jaws) → review+
https://hg.mozilla.org/mozilla-central/rev/865b5e559c13
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Confirming the fix using latest 44.0a2 Aurora build on a Microsoft Surface Pro 2 device running Win10 64-bit.
Status: RESOLVED → VERIFIED
See Also: → 1234852
See Also: 1234852
You need to log in before you can comment on or make changes to this bug.