Firefox browser windows reorder when Windows 10 theme/accent color changes, if normal window(s) is minimized or maximized once. Thunderbird/SeaMonkey has same issue too.

RESOLVED FIXED in Firefox 52

Status

()

Core
Widget: Win32
P1
normal
RESOLVED FIXED
2 years ago
2 months ago

People

(Reporter: screndib, Assigned: Hal Gentz)

Tracking

43 Branch
mozilla53
x86_64
Windows 10
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr45- wontfix, thunderbird_esr4551+ fixed, firefox50 wontfix, firefox51 wontfix, firefox52+ fixed, firefox53 fixed)

Details

(Whiteboard: tpi:+)

Attachments

(3 attachments, 4 obsolete attachments)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20151217102820

Steps to reproduce:

1. Open Firefox.
2. Open a new Firefox window. (2 Firefox windows are now open)
3. Minimize both Firefox windows to the task bar.
4. Restore both Firefox windows from the task bar.
5. Open the Windows 10 "Personalization" settings page (Start->type "personalization").
6. Select the "Colors" section from the left.
7. Toggle any of the options, or change accent colors.

I have observed this same behavior in 43, 44, and 45.


Actual results:

The two Firefox windows will change their relative z-order whenever a setting changes.  Some settings will cause this to happen multiple times which results in flicker.

This is a particularly annoying issue because it occurs whenever the personalization settings change regardless of the reason.  For example, if you have your desktop background set to "Slideshow" and "Automatically pick an accent color from my background" set to on, then the Firefox window reordering will unexpectedly happen whenever the desktop background changes.


Expected results:

The Firefox windows should remain in their existing order.
(Reporter)

Updated

2 years ago
OS: Unspecified → Windows 10
Hardware: Unspecified → x86_64
(Reporter)

Comment 1

2 years ago
Created attachment 8700813 [details] [diff] [review]
patch.diff

Making an attempt at a fix.  This appears to resolve the issue, but I don't know enough about the internals to know whether or not it has any unintended side effects.

Comment 2

2 years ago
WFM on Fx44b1 Win10, but I'm not sure I understand the steps to reproduce.
(Reporter)

Comment 3

2 years ago
Here are some slightly expanded and reordered repro steps that are hopefully more clear, if there's anything else I can add that would help just let me know.  I also realized Start->type "personalization" also opened the wrong settings page so I updated that.

Steps to reproduce:

1. Open Firefox.
2. Open a new Firefox window. (e.g. Ctrl+N, 2 Firefox windows are now open)
3. Ensure that the Firefox windows are overlapping so you can see the issue occur.
4. Open the Windows 10 "Personalization" settings page (Right click on desktop->Select "Personalize").  Make sure it's not covering the Firefox windows so you can see the issue occur.
5. Select the "Colors" section from the left side of the Personalization dialog.
6. Toggle any of the options (e.g. turn "Make Start, taskbar, and action center transparent" on and off), or change accent colors.  Observe that the Firefox windows behave correctly (i.e. they seem entirely unaffected)
7. Minimize both Firefox windows to the task bar using the standard "-" button at the top right of the window.
8. Restore both Firefox windows by selecting them in the task bar.  They should restore such that they're still overlapped.
9. Toggle the Windows personalization options as you did in step 4.  Each time one of the Windows options is toggled the Firefox windows will switch focus and reorder themselves, sometimes multiple times rapidly.

Updated

2 years ago
Component: Untriaged → Widget: Win32
Product: Firefox → Core

Updated

2 years ago
Attachment #8700813 - Flags: review?(jmathies)

Comment 4

2 years ago
(In reply to screndib from comment #3)

Nice, I reproduce the problem now.
Assignee: nobody → screndib
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Comment 5

2 years ago
Comment on attachment 8700813 [details] [diff] [review]
patch.diff

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

::: widget/windows/nsWindow.cpp
@@ +6151,5 @@
>  
> +	  // Windows 10 may send WM_WINDOWPOSCHANGING when the theme changes which 
> +	  // can end up triggering this function on normal size windows that are in 
> +	  // the background.  Update mLastSizeMode here to prevent unexpected focus.
> +	  mLastSizeMode = mSizeMode;

if this is windows 10 and up can we gate this on an os check? This code is very touchy. I can see a small change like this breaking something on older operating systems.

also there's whitespace in your commenting.

Did you test on other os to see if this regresses anything?
Attachment #8700813 - Flags: review?(jmathies) → review-
Blocks: 1258152
Comment hidden (me-too)
(Assignee)

Comment 7

a year ago
I can confirm this bug happens on Firefox nightly v51.0a1 when running Windows 10 with a background which changes.

I've been wondering why Firefox steals the focus every 1 minute, and I can't wait tell this bug is fixed.
Until then I'll just disable my background. :(
(Assignee)

Comment 8

a year ago
Hello folks, I need your aid in this matter:

Currently I'm using this code:
if (mWidgetListener && mSizeMode != previousSizeMode)
      mWidgetListener->SizeModeChanged(mSizeMode);

    // If window was restored, window activation was bypassed during the 
    // SetSizeMode call originating from OnWindowPosChanging to avoid saving
    // pre-restore attributes. Force activation now to get correct attributes.
    if (mLastSizeMode != nsSizeMode_Normal && mSizeMode == nsSizeMode_Normal) {
      DispatchFocusToTopLevelWindow(true);
	  
	  // Windows 10 may send WM_WINDOWPOSCHANGING when the theme changes which 
	  // can end up triggering this function on normal size windows that are in 
	  // the background.  Update mLastSizeMode here to prevent unexpected focus.
	  //
	  // Works on Windows 2000 and up
	  // See https://msdn.microsoft.com/en-us/library/ms724833(v=vs.85).aspx#Remarks
	  // Also: https://msdn.microsoft.com/en-us/library/ms724451(v=vs.85).aspx
	  
	  OSVERSIONINFO osvi;

	  ZeroMemory(&osvi, sizeof(OSVERSIONINFO));
	  osvi.dwOSVersionInfoSize = sizeof(OSVERSIONINFO);

	  GetVersionEx(&osvi);

	  if(osvi.dwMajorVersion >= 5)
	    mLastSizeMode = mSizeMode;
	}

GetVersionEx works on versions of windows at or above Windows 2000, one minor issue is that its been deprecated for Windows 8+, While it appears to works for Windows 10 right now, I'm concerned it might stop working.

Its been replaced with this series of functions: https://msdn.microsoft.com/en-us/library/dn424972(v=vs.85).aspx

Sadly IsWindows10OrGreater's minimum OS is Windows 10, I'm not aware if calling IsWindows10OrGreater on a version of windows before Win10 will cause a error or what exactly.

Hopefully someone here can advice me on what to do.
(Assignee)

Comment 9

a year ago
Created attachment 8787978 [details] [diff] [review]
Possible patch. Not reviewed by anyone.

Updated

a year ago
Attachment #8787978 - Flags: review?(jmathies)

Comment 10

a year ago
Jim, can you assign a moz reviewer if you can't, please.

Updated

a year ago
Attachment #8700813 - Attachment is obsolete: true

Updated

a year ago
Flags: needinfo?(screndib)
Hmm, I can't repo on Win 8.1. Will try 10.
Flags: needinfo?(jmathies)
(Assignee)

Comment 12

a year ago
Jim, as far as I'm aware, the bug only happens on win 10. If your unable to reproduce it on windows 10 I could make a .gif showing how.

Updated

a year ago
Flags: needinfo?(jmathies)
Comment on attachment 8787978 [details] [diff] [review]
Possible patch. Not reviewed by anyone.

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

::: widget/windows/nsWindow.cpp
@@ +6256,1 @@
>        DispatchFocusToTopLevelWindow(true);

AFAICT DispatchFocusToTopLevelWindow is still called once, so don't we still have a focus problem here?

Updated

a year ago
Priority: -- → P1
Whiteboard: tpi:+
(Assignee)

Comment 14

a year ago
When I tested it I found setting mLastSizeMode to mSizeMode stopped the order from changing. 
I don't know why it does that, you should ask screndib (as he is the one who originally proposed adding that line).
(Assignee)

Comment 15

a year ago
Jim, Windows 10 sends a WM_WINDOWPOSCHANGING for every window, so if we have 3 firefox windows it will send a WM_WINDOWPOSCHANGING for the top Window, then to the window bellow it, and then to the window below that, ext. Util it reaches the bottom window.

As a result, DispatchFocusToTopLevelWindow will be called latter the more bottom the window is which causes the order of the firefox windows to invert (window at top is now at he bottom, ext).

By setting mLastSizeMode to mSizeMode we insure DispatchFocusToTopLevelWindow only gets called once for the top window. For all other firefox windows, mLastSizeMode will now equal mSizeMode causing this block to never be called.

Updated

11 months ago
Blocks: 1306773

Updated

11 months ago
Duplicate of this bug: 1306773
(Assignee)

Updated

10 months ago
Flags: needinfo?(jmathies)
No longer blocks: 1306773
Created attachment 8803645 [details]
Steps to reproduce, test result with "manual accent color chsnge" and "Desktop backgroung slideshow"

Attached contains;
- STR derived from STR in Comment #2 by bug opener
- Some test results with "multiple Firefox windows", "multiple Thunderbird windows", and "multiple Firefox windows" + "multiple Thunderbird windows" at same time, using "manual accent color change"(STR of this bug) and "automatic/periodical accent color change utilizing Desktop background slideshow"(STR of bug 1258152).
Proposed change.
>      // If window was restored, window activation was bypassed during the 
>      // SetSizeMode call originating from OnWindowPosChanging to avoid saving
>      // pre-restore attributes. Force activation now to get correct attributes.
> -    if (mLastSizeMode != nsSizeMode_Normal && mSizeMode == nsSizeMode_Normal)
> +    if (mLastSizeMode != nsSizeMode_Normal && mSizeMode == nsSizeMode_Normal) {
>        DispatchFocusToTopLevelWindow(true);
>  
> +      // Windows 10 may send WM_WINDOWPOSCHANGING when the theme changes which
> +      // can end up triggering this function on normal size windows that are in
> +      // the background.  Update mLastSizeMode here to prevent unexpected focus.
> +      LPBYTE pinfoRawData;
> +      if (NERR_Success == NetWkstaGetInfo(NULL, 100, &pinfoRawData)) {
> +        WKSTA_INFO_100 * pworkstationInfo = (WKSTA_INFO_100 *)pinfoRawData;
> +        ::NetApiBufferFree(pinfoRawData);
> +
> +        if(pworkstationInfo->wki100_ver_major >= 10) {
> +          mLastSizeMode = mSizeMode;
> +        }
> +      }
> +    }
>      // Skip window size change events below on minimization.
>      if (mSizeMode == nsSizeMode_Minimized)
>        return;
>    }

My STR does do "accent color change" only.
I think sent message by Win10 in this case is WM_DWMCOLORIZATIONCOLORCHANGED message onlu.
  See https://msdn.microsoft.com/en-us/library/windows/desktop/dd388198%28v=vs.85%29.aspx
Is it wrong?
Does Win10 actually send WM_WINDOWPOSCHANGING to windows even when "accent color change" only?

> +    if (mLastSizeMode != nsSizeMode_Normal && mSizeMode == nsSizeMode_Normal) {
>        DispatchFocusToTopLevelWindow(true);
>        => Added logic follows
>      }
In test by bug opener and in test by me, any Firefox/Thunderbird window was kept as "Non-maximized/non-minimized Active/Foreground/HasKeyborardFocus window" or
 "Non-maximized/non-minimized non-Active/non-Foreground/DoesNotHaveKeyborardFocus window".
"Minimize to Taskbar, restore from Taskbar" is done only once before startring test.
So, I think (mLastSizeMode == nsSizeMode_Normal && mSizeMode == nsSizeMode_Normal) is always true exept when first "accent color change". in my tests.
i.e.  (mLastSizeMode != nsSizeMode_Normal && mSizeMode == nsSizeMode_Normal) is false in my tests.

Will above "added logic" surely resolve problem of this bug which was observed in my tests?

If WM_WINDOWPOSCHANGING and WM_WINDOWPOSCHANGED are sent during my tests, I guess that it's produced by "unexpexted/unwanted Active window swich or Focused windw switch or z-index change by Firefox/Thunderbird upn accent color change".
Please see my test results. 
"Order of Firefox windows" and/or "order of Thunderbird windows" was changed to reverted order after "accent color change". 

Or MS windows 10 always sends WM_WINDOWPOSCHANGING and WM_WINDOWPOSCHANGED in addition to WM_THEMECHANGED or WM_DWMCOLORIZATIONCOLORCHANGED?

 .
It looks for me that Firefox/Thunderbird requests "make me top window in my family" when WM_DWMCOLORIZATIONCOLORCHANGED happens.
 current order : Win1 -> Win2 -> Win3 -> Win4, WM_DWMCOLORIZATIONCOLORCHANGED is sent in this order
 1.  Win1 requests "make me top window in my family" (similar to mouse click at this window)
 2.  Win2 requests "make me top window in my family" (similar to mouse click at this window),
 3.  Win3 requests "make me top window in my family" (similar to mouse click at this window)
 4.  Win4 requests "make me top window in my family" (similar to mouse click at this window),
 => window order is reverted : Win4 -> Win3 -> Win2 -> Win1
If event like above happens, WM_WINDOWPOSCHANGING and WM_WINDOWPOSCHANGED is sent to all other windows in family upon each event like above, as a result of "window order change request". like above.
(In reply to Ahmed (Gentz) El Gendy from comment #14)
> When I tested it I found setting mLastSizeMode to mSizeMode stopped the order from changing. 

In my test, it looked that (X) "minimize Firefox/Thunderbird window then restore from Taskbar once" was mandatry to observe "reverting window order when accent color change".
The step (X) is not written in STR of Comment #0 by bug opener, but the step (X)  is stated in modified STR of Comment #2 by bug opener..

mLastSizeMode won't be set to nsSizeMode_Normal once Firefox/Thunderbird window is minimized on Win10?
Created attachment 8803685 [details]
Log of focus switch among Messenger/ConfigEditor/ActivityManager/Composer by "accent color change by Slideshow" with NSPR_LOG_MODULES=WidgetFocus:5,Focus:5

Test with STR stated in previous attachment.

For ease of looking log, following windows of Thunderbird were used.
1. Messenger chrome://messenger/content/messenger.xul
2. ConfigEditor chrome://global/content/config.xul
3. ActivityManager chrome://messenger/content/activity.xul
4. Composer chrome://messenger/content/messengercompose/messengercompose.xul

(0) Slideshow with "accent color from background image".
     4 Thunderbird windows are minimized and restored from Taskbar once.
     Repeated some tests, including "mouse click at each window".
(1) Click Messenger, ConfigEditor, ActivityManager, Composer in this order.
=> window order is:
    Composer  -> ActivityManager -> ConfigEditor -> Messenger
(2) accent color is changed by Slideshow+"accent color from background image".
(3) Log shows 3times swich of focus among 4 windows.
    Composer  -> ActivityManager -> ConfigEditor -> Messenger
    -> Composer  -> ActivityManager -> ConfigEditor -> Messenger
    -> Composer  -> ActivityManager -> ConfigEditor -> Messenger
(4) Finally, windows are ordered:
  Messenger -> ConfigEditor -> ActivityManager -> Composer
  i.e. Reversed window order from order before accent color change.

I guess;
 1-st focus switch is by event of "accent color change", 
 2-nd focus swich is by "window order change produced by 1-st focus swich", 
 3-rd focus switch is by "window order change produced by 2-nd focus swich".
Attachment #8803685 - Attachment description: Log for focus switch among Messenger/ConfigEditor/ActivityManager/Composer by "accent color change by Slideshow" with NSPR_LOG_MODULES=WidgetFocus:5,Focus:5 → Log of focus switch among Messenger/ConfigEditor/ActivityManager/Composer by "accent color change by Slideshow" with NSPR_LOG_MODULES=WidgetFocus:5,Focus:5
Current OnWindowPosChanged.
  if (mLastSizeMode != nsSizeMode_Normal && mSizeMode == nsSizeMode_Normal)
       DispatchFocusToTopLevelWindow(true);

If WM_WINDOWPOSCHANGED is sent by Win10 upon "accent color change" on Win10,
and if WM_WINDOWPOSCHANGED is sent due to window order change by Firefox/Thunderbird upon(or after) "accent color change",
and if who changed window order is above calling of DispatchFocusToTopLevelWindow(true) due to mLastSizeMode!=nsSizeMode_Normal
(Q1) Why window order change won't occur in Win8.1 and Win7 upon "accent color change"?
(Q2) Why focus(perhaps keyboard focus) is set to this window who called DispatchFocusToTopLevelWindow(true) if Win10?
       Why the focus is not set to current Topmost window in Firefox/Thunderbird(Active/Foreground window usually) ?
       DispatchFocusToTopLevelWindow(true) means "set keyboard focus to this window"?
       Or definition of TopLevelWindow which DispatchFocusToTopLevelWindow assumes is changed in Win10 from Win8.1/Win7?
       Or DispatchFocusToTopLevelWindow(true) in Win8.1/Win7 didn't change target window to Active/Foreground window,
       but DispatchFocusToTopLevelWindow(true) in Win10 changes the target window to Active/Foreground window? 

If cause is mLastSizeMode!=nsSizeMode_Normal of our "Restored window from minimized state",
(Q3) Why window order change won't occur in Win8.1 and Win7 upon "accent color change"?
(Q4) Where is mLastSizeMode!=nsSizeMode_Normal of our "Restored window from minimized state" cleared?
I tried resize of our windows, maximize/restore to ordinal window. But window order change still occurred on Win 10.
Is mLastSizeMode=mSizeMode of proposed patch is only one place for clearing mLastSizeMode!=nsSizeMode_Normal of restored from minimized window?
(Assignee)

Comment 23

10 months ago
Lets say we have Win1, Win2, Win3 (Win 1 in foreground), IF accent color changes AND background changes the order will become Win3 -> Win2 -> Win1. 
IF ONLY the accent color changes (same background) then each window will flciker and try to get ontop of the other window for a secound or 2, then a random window will end up on top.
If you open Windows Explorer on windows 10 you'll notice whenever the window order changes on firefox the explorer's scroll will reset. (See: http://answers.microsoft.com/en-us/windows/forum/windows_10-files/folder-files-reset-exactly-when-slideshow-pictures/dc109c86-43e1-4071-bca9-bf7a018f73df)

(Q1) This is a windows 10 thing, someone at Microsoft derped up things.
(Q3) See Q1.
(Q4) The only other place mLastSizeMode is changed is on line 1869 of nsWindow, in function "void nsWindow::SetSizeMode(nsSizeMode aMode)". Calling SetSizeMode(mSizeMode)instead of running "mLastSizeMode = mSizeMode" DOES NOT fix the issue. This is because aMode == mSizeMode, so it returns before updating mLastSizeMode.

IF we don't add the proposed code on lines 6265 - 6277 but instead go to the SetSizeMode function and put mLastSizeMode = mSizeMode before that if statement then the bug STILL happens.

I'll answer Q2 later.
(In reply to Ahmed (Gentz) El Gendy from comment #23)

Thanks for quick reply.
By your proposed patch and explanation, I'm aware of following.
- If "accent color change produces WM_WINDOWPOSCHANGING and WM_WINDOWPOSCHANGED in WIN10 ONLY"
  is assumed, phenomenon of this bug, Bug 1258152, are explained well.
- Further, if this bug and Bug 1258152 are explained well,
  phenomenon of Bug 942610 is also explained pretty well.

Please see Bug 942610 Comment #117 and Bug 942610 Comment #119.
Because Bug 942610 is not Win10 only problem, "+ mLastSizeMode = mSizeMode;" of your patch or equivalent is probably mandatory even when non-Win10.

I think following are needed.
(1) Problem of that no one clears mLastSizeMode==nsSizeMode_Minimized state once it's set by minimize window.
(1-1) nsWindow::OnWindowPosChanging
Do mLastSizeMode=mSizeMode always, even if it's meaningless when "first restore from minimized".
This is required for "event notification of Z-Order change"
which is sent to all foreground/background windows(window of mSizeMode==nsSizeMode_Normal).
(1-2) nsWindow::OnWindowPosChanged
  if (mLastSizeMode != nsSizeMode_Normal && mSizeMode == nsSizeMode_Normal)
       DispatchFocusToTopLevelWindow(true);
There is no need of change at here, if change of (A-1) is made,
and if window is not changed to minimized state by Win10 temporarily before event notification by accent color change.
(2) Improvement or change in DispatchFocusToTopLevelWindow.
This code seems to assume only one eWindowType_toplevel window.
But it may be wrong. Multiple eWindowType_toplevel windows seems to exist in a Thunderbird instance on recent MS Windows. 
If so, main culprit of focus change/steal is DispatchFocusToTopLevelWindow.
Current code probably changes "this window" to Active window instead of "top most window among a Thunderbird instance".
(Assignee)

Comment 25

10 months ago
Created attachment 8804935 [details] [diff] [review]
We now make nLastSizeMode = mSizeMode every time
(In reply to Ahmed (Gentz) El Gendy from comment #25)
> We now make nLastSizeMode = mSizeMode every time

I prefer code like next.
    if (mLastSizeMode != mSizeMode){  // note: this is not mLastSizeMode != nsSizeMode_Normal
       // SizeMode change happened
       // if cgange to Normal mode, move focus to toplevel window instead of child window
       if (mSizeMode == nsSizeMode_Normal) DispatchFocusToTopLevelWindow(true);
       // Clear mLastSizeMode != mSizeMode state
       mLastSizeMode = mSizeMode;
    }
No need to do memory update upon each WM_WINDOWPOSCHANGED event, even if the event is not so frequently invoked.

Will problem on Win in Bug 942610 be resolved by your change?

I guess so, if problem occurs only when messenger window was minimized once.
I can not imagine other reason why problem ccurs in limited environment only, than "messenger window was minimized once".
If DispatchFocusToTopLevelWindow(true) is not called at Messenger/Composer when popup window is closed.
I think focus will be returned to window who had focus previously.
(Assignee)

Comment 27

10 months ago
I think it good as is.

I don't use Thunderbird, so I wouldn't know.

As for suggestion number 2, do you mean that in thunderbird a eWindowType_toplevel might have a parent which is also a eWindowType_toplevel? Or did you mean something else?
(In reply to Ahmed (Gentz) El Gendy from comment #27)
> As for suggestion number 2, do you mean that in thunderbird a
> eWindowType_toplevel might have a parent which is also a
> eWindowType_toplevel? Or did you mean something else?

Sorry, I misunderstood purpose of DispatchFocusToTopLevelWindow(true).
I thought it's to move focus to top most windoew withun a Firefox/Thunderbird/SeaMonkey instance.
This was merely "move focus to toplevel window instead of child window within a toplevel window". i.e. Move focus to me.
As known by code, problem occurs on both "normal window minimized once" and "normal window maximized once".
Summary: Firefox browser windows reorder when Windows 10 theme changes. → Firefox browser windows reorder when Windows 10 theme/accent color changes, if normal window(s) is minimized or maximized once. Thunderbird/SeaMonkey has same issue too.
Attachment #8804935 - Flags: review?(epinal99-bugzilla2)
(In reply to Ahmed (Gentz) El Gendy from comment #25)
> Created attachment 8804935 [details] [diff] [review]
> We now make nLastSizeMode = mSizeMode every time

This bug was opened on 2015-12-21, and essentially absolutely same patch as yours was proposed by Comment #1 on 2015-12-21.
Why solution of this bug was not landed for so long time?
Attachment #8804935 - Flags: review?(jmathies)

Comment 31

10 months ago
Comment on attachment 8804935 [details] [diff] [review]
We now make nLastSizeMode = mSizeMode every time

I'm not a reviewer.
Attachment #8804935 - Flags: review?(epinal99-bugzilla2)

Comment 32

10 months ago
(In reply to WADA from comment #30)
> (In reply to Ahmed (Gentz) El Gendy from comment #25)
> > Created attachment 8804935 [details] [diff] [review]
> > We now make nLastSizeMode = mSizeMode every time
> 
> This bug was opened on 2015-12-21, and essentially absolutely same patch as
> yours was proposed by Comment #1 on 2015-12-21.
> Why solution of this bug was not landed for so long time?

Because the patch posted was untested and not well diagnosed, and I haven't had the time (like you have) to dig into this for a proper fix.

Thanks for working on it.
Flags: needinfo?(jmathies)

Updated

10 months ago
Attachment #8787978 - Flags: review?(jmathies)

Comment 33

10 months ago
Comment on attachment 8804935 [details] [diff] [review]
We now make nLastSizeMode = mSizeMode every time

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

This looks good.
Attachment #8804935 - Flags: review?(jmathies) → review+

Updated

10 months ago
Assignee: screndib → m-wada
(Assignee)

Updated

10 months ago
Attachment #8787978 - Attachment is obsolete: true
(Assignee)

Comment 34

10 months ago
Created attachment 8807919 [details] [diff] [review]
Updated the title
Attachment #8804935 - Attachment is obsolete: true
(Assignee)

Comment 35

10 months ago
Can someone push it to the try server or something?

Updated

9 months ago
Assignee: m-wada → zegentzy

Comment 36

9 months ago
Created attachment 8810525 [details] [diff] [review]
patch

try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f7c885e1481ae3b4233174eabe28f7f719523759
Attachment #8807919 - Attachment is obsolete: true

Updated

9 months ago
Attachment #8810525 - Flags: review+

Updated

9 months ago
Keywords: checkin-needed
(In reply to Jim Mathies [:jimm] from comment #36)
> try push:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=f7c885e1481ae3b4233174eabe28f7f719523759

I clicked "cl" in "M( cl oth )" of following.
> Windows 7 opt 	M( cl oth ) M-e10s( ... )
And I got following zip.
> https://archive.mozilla.org/pub/firefox/try-builds/jmathies@mozilla.com-f7c885e1481ae3b4233174eabe28f7f719523759/try-win32/firefox-53.0a1.en-US.win32.zip
Is this correct one for testing on Windows 10?

With above build, I couldn't observe Firefox's window order change phenomenon on Win 10 when accent color was manually changed.
"Scroll position reset of Windows file explorer" was consistently observed by the manual accent color change.

Comment 38

9 months ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2778571f6f34
Update mLastSizeMode when window size mode changes within Windows widget, avoids a window focus problem on Windows 10 with theme changes. r=jimm
Keywords: checkin-needed

Comment 39

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2778571f6f34
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
No need to port this fix to esr?
status-firefox53: fixed → ---
status-firefox-esr45: --- → affected
status-thunderbird_esr45: --- → affected
tracking-firefox-esr45: --- → ?
tracking-thunderbird_esr45: --- → ?
Has STR: --- → yes

Comment 41

9 months ago
[Tracking Requested - why for this release]: This has been a constant annoyance to both Thunderbird and Firefox users, so it would be great if this could land in m-c at gecko 52.
tracking-firefox52: --- → ?

Comment 42

9 months ago
https://hg.mozilla.org/mozilla-central/rev/2778571f6f34
as far as I can tell this doesn't match the criteria for ESR, not tracking
status-firefox-esr45: affected → wontfix
tracking-firefox-esr45: ? → -
(In reply to Kent James (mostly AFK 2016-11-19 to 2016-11-26) (:rkent) from comment #41)
> [Tracking Requested - why for this release]: This has been a constant
> annoyance to both Thunderbird and Firefox users, so it would be great if
> this could land in m-c at gecko 52.

Just to be clear, in Thunderbird this is more than an annoyance. Thunderbird has many single key shortcuts that alter messages, and of course delete key deletes messages.  So window switching has distructive power.


> as far as I can tell this doesn't match the criteria for ESR, not tracking

In light of the above, IMO this is worthy of reconsideration for ESR 45.
(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #44)
> Just to be clear, in Thunderbird this is more than an annoyance. Thunderbird
> has many single key shortcuts that alter messages, and of course delete key
> deletes messages.  So window switching has distructive power.

1. Thunderbird messenger window is minimized once, and current state = normal window.
   Thread pane is usually focused and mail is usually selected, because user usually views mail.
   In this context, Delete key==delete currently selected mail(s).
2. Composer window is opened. It freequently occurs, because Thunderbird is mailer.
   And Delete key is frequently used while mail composing, because mail composition=text editing.
3. If this bug happens when user tries to press Delete key at composer window,
   Delete key is passed to messenger window, and a mail or some mails are deleted.
In SeaMonkey, "ask before delete mail" is enabled by default, so mail is usually protected by alert when 3. happened.
However, IIRC, in Thunderbird, ordinal dDelete key(no Shift modifier) == delete mail immediately.
So, this bug can be called "powerful tool to produce unwanted deleted mails phenomenon" in Thunderbird.

> > as far as I can tell this doesn't match the criteria for ESR, not tracking 
> In light of the above, IMO this is worthy of reconsideration for ESR 45.

As known by patch, "essentially only one liner change" is applied to following.
  diff --git a/widget/windows/nsWindow.cpp b/widget/windows/nsWindow.cpp
No way to apply the change to library contains nsWindow.cpp which is used by building Thunderbird esr 45?

By the way, when SeaMonkey, following occurs by this bug, because SeaMonkey = Browser + MailNews.
 1. If user uses MailNews, Browser comes front of MailNews by this bug.
 2. If user uses Browser, MailNews comes front of Browser by this bug.
If user sets Interval=1 minute at Win10's slide show setting, user can experience above two phenomenon every 1 minute :-)

Comment 46

9 months ago
Wayne, we'll take this probably in THUNDERBIRD_45_VERBRANCH since our acceptance criteria are more aggressive than for Firefox.

Updated

9 months ago
status-firefox50: --- → wontfix
status-firefox51: --- → wontfix
status-firefox52: --- → affected
status-firefox53: --- → fixed

Comment 47

9 months ago
Comment on attachment 8810525 [details] [diff] [review]
patch

Approval Request Comment
[Feature/Bug causing the regression]:
bug 506815
[User impact if declined]:
annoying issue with video fullscreen
[Is this code covered by automated tests?]:
yes
[Has the fix been verified in Nightly?]:
yes
[Needs manual test from QE? If yes, steps to reproduce]: 
no, community tested.
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:
moderate, aurora uplift should be fine.
[Why is the change risky/not risky?]:
this changes widget code which can exhibit unexpected side effects when modified.
[String changes made/needed]:
none
Attachment #8810525 - Flags: approval-mozilla-aurora?
Comment on attachment 8810525 [details] [diff] [review]
patch

fix window focus issue on windows, verified on nightly, take in aurora52
Attachment #8810525 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 49

9 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/4f3edfcbc9ba
status-firefox52: affected → fixed
tracking-firefox52: ? → +

Comment 50

7 months ago
Pushed to THUNDERBIRD_45_VERBRANCH https://hg.mozilla.org/releases/mozilla-esr45/rev/eb6f18dbfe8d
status-thunderbird_esr45: affected → fixed
tracking-thunderbird_esr45: ? → 51+

Updated

6 months ago
Duplicate of this bug: 1258152
Setting qe-verify- based on Jim's assessment on manual testing needs (see Comment 47) and the fact that this fix has automated coverage.
Flags: qe-verify-
Duplicate of this bug: 1334855
You need to log in before you can comment on or make changes to this bug.