1336230 - When a window is maximized, it should not show the window opening transition

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

As part of our efforts to improve perceived performance, one of the things that the UX team has identified coming out of the Photon work is that we want the opening of new windows to feel smooth.

On Windows, at least, there is a default "effect" that makes the window "fade in" and zoom. At least for maximized windows, the UX team feels that this effect just takes valuable time and can get axed.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

I've looked at this briefly, and I _suspect_ it's pretty straight forward. Gonna try this today.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1336230 - Rename macsuppressanimation and CHROME_MAC_SUPPRESS_ANIMATION to be platform agnostic.

Review commit: https://reviewboard.mozilla.org/r/109728/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/109728/

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1336230 - If the current window is maximized if the user opens a new window, skip the opening animation.

Review commit: https://reviewboard.mozilla.org/r/109730/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/109730/

Comment 4

[Mike Conley (:mconley) (:⚙️)]

So we already had a feature flag for windows to prevent animation on MacOS. I've made that agnostic, and written the JavaScript that checks the window state for STATE_MAXIMIZED and adds that feature flag if the window is indeed maximized.

Basically, what I've got here is the behaviour that's been described, but on MacOS. Not sure if we want it there, but that's where I started. Now I'm going to try to get the suppressanimation feature supported for our Windows backend.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Do we want this change on MacOS as well as Windows?

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1336230 - Add suppressanimation support to Windows backend.

Review commit: https://reviewboard.mozilla.org/r/109748/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/109748/

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8833479 [details]
Bug 1336230 - Rename macsuppressanimation and CHROME_MAC_SUPPRESS_ANIMATION to be platform agnostic.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/109728/diff/1-2/

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8833480 [details]
Bug 1336230 - If the current window is maximized if the user opens a new window, skip the opening animation.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/109730/diff/1-2/

Comment 9

[:Felipe Gomes (needinfo for replies!)]

Is that the same effect used on browser startup when sessionrestore is restoring the windows?

Comment 10

[Mike Conley (:mconley) (:⚙️)]

(In reply to :Felipe Gomes (needinfo me!) from comment #9)
> Is that the same effect used on browser startup when sessionrestore is
> restoring the windows?

Yes. A consequence (or benefit?) of the patches here is that on Windows, we'll stop animating the windows that are opened on session restore.

Comment 11

[Jim Mathies [:jimm]]

Comment on attachment 8833479 [details]
Bug 1336230 - Rename macsuppressanimation and CHROME_MAC_SUPPRESS_ANIMATION to be platform agnostic.

https://reviewboard.mozilla.org/r/109728/#review111124

Comment 12

[Jim Mathies [:jimm]]

Comment on attachment 8833524 [details]
Bug 1336230 - Add suppressanimation support to Windows backend.

https://reviewboard.mozilla.org/r/109748/#review111128

Comment 13

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8833480 [details]
Bug 1336230 - If the current window is maximized if the user opens a new window, skip the opening animation.

https://reviewboard.mozilla.org/r/109730/#review110862

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/48c77c39e587
Rename macsuppressanimation and CHROME_MAC_SUPPRESS_ANIMATION to be platform agnostic. r=jimm
https://hg.mozilla.org/integration/autoland/rev/768bf846fac5
Add suppressanimation support to Windows backend. r=jimm
https://hg.mozilla.org/integration/autoland/rev/16dc1c594448
If the current window is maximized if the user opens a new window, skip the opening animation. r=Felipe

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/48c77c39e587
https://hg.mozilla.org/mozilla-central/rev/768bf846fac5
https://hg.mozilla.org/mozilla-central/rev/16dc1c594448

Comment 16

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Looks good!
As just discussed in person, let's enable the fix on all platforms.

Comment 17

[Frank-Rainer Grahl (:frg)]

In toolkit/components/windowwatcher/test/browser_new_content_window_chromeflags.js

> -  "macsuppressanimation": {
> +  "suppressanimation": {
>      flag: Ci.nsIWebBrowserChrome.CHROME_MAC_SUPPRESS_ANIMATION,

Shouldn't this be:

> -  "macsuppressanimation": {
> -    flag: Ci.nsIWebBrowserChrome.CHROME_MAC_SUPPRESS_ANIMATION,
> +  "suppressanimation": {
> +    flag: Ci.nsIWebBrowserChrome.CHROME_SUPPRESS_ANIMATION,

Comment 18

[Mike Conley (:mconley) (:⚙️)]

(In reply to Frank-Rainer Grahl from comment #17)
> In
> toolkit/components/windowwatcher/test/browser_new_content_window_chromeflags.
> js
> 
> > -  "macsuppressanimation": {
> > +  "suppressanimation": {
> >      flag: Ci.nsIWebBrowserChrome.CHROME_MAC_SUPPRESS_ANIMATION,
> 
> Shouldn't this be:
> 
> > -  "macsuppressanimation": {
> > -    flag: Ci.nsIWebBrowserChrome.CHROME_MAC_SUPPRESS_ANIMATION,
> > +  "suppressanimation": {
> > +    flag: Ci.nsIWebBrowserChrome.CHROME_SUPPRESS_ANIMATION,

Yep, definitely. Good eye, thanks. I've filed bug 1337432 to address this.