Last Comment Bug 1336230 - When a window is maximized, it should not show the window opening transition
: When a window is maximized, it should not show the window opening transition
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Win32 (show other bugs)
: 50 Branch
: Unspecified Unspecified
-- normal (vote)
: mozilla54
Assigned To: Mike Conley (:mconley)
:
: Jim Mathies [:jimm]
Mentors:
Depends on:
Blocks: photon 1337432
  Show dependency treegraph
 
Reported: 2017-02-02 14:42 PST by Mike Conley (:mconley)
Modified: 2017-02-07 09:43 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed

MozReview Requests
Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:
Show discarded requests

Attachments
Bug 1336230 - Rename macsuppressanimation and CHROME_MAC_SUPPRESS_ANIMATION to be platform agnostic. (59 bytes, text/x-review-board-request)
2017-02-03 13:15 PST, Mike Conley (:mconley)
jmathies: review+
Details | Review
Bug 1336230 - If the current window is maximized if the user opens a new window, skip the opening animation. (59 bytes, text/x-review-board-request)
2017-02-03 13:15 PST, Mike Conley (:mconley)
felipc: review+
Details | Review
Bug 1336230 - Add suppressanimation support to Windows backend. (59 bytes, text/x-review-board-request)
2017-02-03 14:53 PST, Mike Conley (:mconley)
jmathies: review+
Details | Review

Description User image Mike Conley (:mconley) 2017-02-02 14:42:07 PST
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 User image Mike Conley (:mconley) 2017-02-03 13:00:09 PST
I've looked at this briefly, and I _suspect_ it's pretty straight forward. Gonna try this today.
Comment 2 User image Mike Conley (:mconley) 2017-02-03 13:15:30 PST Comment hidden (mozreview-request)
Comment 3 User image Mike Conley (:mconley) 2017-02-03 13:15:30 PST Comment hidden (mozreview-request)
Comment 4 User image Mike Conley (:mconley) 2017-02-03 13:19:03 PST
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 User image Mike Conley (:mconley) 2017-02-03 13:43:24 PST
Do we want this change on MacOS as well as Windows?
Comment 6 User image Mike Conley (:mconley) 2017-02-03 14:53:43 PST Comment hidden (mozreview-request)
Comment 7 User image Mike Conley (:mconley) 2017-02-03 14:53:43 PST Comment hidden (mozreview-request)
Comment 8 User image Mike Conley (:mconley) 2017-02-03 14:53:43 PST Comment hidden (mozreview-request)
Comment 9 User image :Felipe Gomes (needinfo me!) 2017-02-04 12:03:26 PST
Is that the same effect used on browser startup when sessionrestore is restoring the windows?
Comment 10 User image Mike Conley (:mconley) 2017-02-04 14:46:49 PST
(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 User image Jim Mathies [:jimm] 2017-02-06 08:36:29 PST
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 User image Jim Mathies [:jimm] 2017-02-06 08:37:19 PST
Comment on attachment 8833524 [details]
Bug 1336230 - Add suppressanimation support to Windows backend.

https://reviewboard.mozilla.org/r/109748/#review111128
Comment 13 User image :Felipe Gomes (needinfo me!) 2017-02-06 11:27:39 PST
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 User image Pulsebot 2017-02-06 13:17:58 PST
Pushed by mconley@mozilla.com:
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 16 User image (Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo 2017-02-07 08:30:04 PST
Looks good!
As just discussed in person, let's enable the fix on all platforms.
Comment 17 User image Frank-Rainer Grahl 2017-02-07 08:36:23 PST
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 User image Mike Conley (:mconley) 2017-02-07 08:56:25 PST
(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.

Note You need to log in before you can comment on or make changes to this bug.