Ciprian Georgiu, Desktop QA Reporter
	Description • 6 years ago

[Affected versions]:
- latest Nightly 63.0a1
- Beta 62.0b16
- Release 61.0.2
- Esr 60.1.1

[Affected platforms]:
- Windows 10 x64, Version 1803 (OS Build 17735.1000) 

[Steps to reproduce]:
1. Launch Firefox.
2. Access the following website and play any video: https://www.youtube.com/ 
3. Drag the Firefox window via mouse, and drop it to the right (or left) side of the screen.
4. Enter Full screen, using the dedicated video button or double click on video.

[Expected result]:
- The video properly enters in full screen, there is no border displayed on the left side and the taskbar is hidden.  

[Actual result]:
- The video does not enter correctly in full screen, the taskbar overlaps the video and a border is displayed on the left side.
 
[Regression range]:
- It doesn't seem to be a recent regression, I was able to reproduce it on 61.0.2 as well. I will investigate further in case of an old regression and provide more info.

[Additional notes]:
- see the attached screenshot
- not repro on the current Windows 10 release
- additionally you can press Windows Key + Left/Right arrow to snap a window

	Jared Wein [:jaws] (please needinfo? me)
	Comment 1 • 6 years ago

This may be an issue with Redstone 5. Can you file an issue with the Feedback Hub to let Microsoft know of the issue?

	Ciprian Georgiu, Desktop QA Reporter
	Comment 2 • 6 years ago

Filed the issue on Feedback Hub as well.

Please note that the issue is not reproducible on Chrome nor Edge.

	Jared Wein [:jaws] (please needinfo? me)
	Comment 3 • 6 years ago

Adam, is this on your radar?

	Adam Gashlin (he/him) [:agashlin] (ex-moco) Assignee
	Comment 4 • 6 years ago

This is the first I've heard of it, I'm getting an RS5 build set up now to investigate.

	:Gijs (he/him)
	Comment 5 • 6 years ago

This smells like bug 1403153. Does the issue go away if you reboot the machine?

	Adam Gashlin (he/him) [:agashlin] (ex-moco) Assignee
	Comment 6 • 6 years ago

It may be related but it's a bigger issue: The taskbar overlaps fullscreen.

I haven't been able to reproduce this yet on 17713, I'm trying to get a newer version.

	Adam Gashlin (he/him) [:agashlin] (ex-moco) Assignee
	Comment 7 • 6 years ago

Confirmed on 17741, investigating.

	Adam Gashlin (he/him) [:agashlin] (ex-moco) Assignee
	Comment 8 • 6 years ago

This seems to be due to a change introduced somewhere between Windows 10 builds 17713 and 17735. It can be reproduced by just snapping the window and hitting F11 for fullscreen.

When we disable window decorations (by clearing WS_CAPTION | WS_THICKFRAME via SetWindowLongPtrW, see HideWindowChrome), we normally get a WS_WINDOWPOSCHANGED with SWP_FRAMECHANGED that just repeats the window position. The window appears to get larger due to the window manager adding an invisible 7 pixels to the left, right, and bottom as long as WC_CAPTION | WS_THICKFRAME are set.

But if the window is snapped then there is another WM_WINDOWPOSCHANGED which gives the position of the window as if it were shrunk by 7 pixels on the right, left, and bottom; this prevents the window from appearing to expand beyond the bounds of the snap region. Only one WM_MOVE and WM_SIZE are generated as a result of the two changes.

The bug seems to be that now, if the window is resized before the second, shrinking WM_WINDOWPOSCHANGED, another shrinking WM_WINDOWPOSCHANGED message will be delivered, but based on the new size that has been requested. So after the resize the window doesn't occupy the whole screen, which throws off the taskbar hiding heuristics.

However it isn't clear why the ITaskbarList2::MarkFullscreenWindow call isn't taking effect, that should be sufficient to draw us over the taskbar anyway (though it won't fix the bars on the sides). Oddly it does work if I only resize to half the full width of the screen...

We could fix this in a few ways, generally we'd have a delay between hiding chrome and resizing the window. Chrome seems to avoid this by always setting the window manager non-client rendering policy to "disabled", this avoids all the issues with disabling window decorations. I don't know if we can directly do that, I'll look into it.

	Ciprian Georgiu, Desktop QA Reporter
	Comment 9 • 6 years ago

(In reply to :Gijs (he/him) from comment #5)
> This smells like bug 1403153. Does the issue go away if you reboot the
> machine?

No, unfortunately the issue is still present.

	Mike Conley (:mconley) (:⚙️)
	Comment 10 • 6 years ago

Is it safe for us to assign this to you for now, :agashlin?

I'm unsure of how to prioritize this, since we don't know the release date of Redstone 5. Out of an abundance of caution, I'm going to P1 this (since Windows is Tier 1 and video sites like YouTube are pretty popular and important), but if that seems to aggressive, please feel free to ratchet it down.

	Adam Gashlin (he/him) [:agashlin] (ex-moco) Assignee
	Comment 11 • 6 years ago

Taken, I'm pretty much treating it as a P1 anyway.

	Adam Gashlin (he/him) [:agashlin] (ex-moco) Assignee
	Comment 12 • 6 years ago

Here's a video showing the variations of the odd behavior, so that I can reference this in a message to the Microsoft list.

	Adam Gashlin (he/him) [:agashlin] (ex-moco) Assignee
	Comment 13 • 6 years ago

Source to the demo

	Adam Gashlin (he/him) [:agashlin] (ex-moco) Assignee
	Comment 14 • 6 years ago

I'm moving this to P2 because:
- I don't think snapping is used very widely
- This affects Microsoft apps as well (at least IE and Console Window), so it has a good chance of being fixed

But I'll keep the ni? so it stays on my radar to check again as we near RS5 release, whenever that will be. I do have a workaround but I'd rather avoid any more complexity in fullscreen.

	pringo4
	Comment 17 • 6 years ago

I am experiencing this bug in the release version of Windows 10 1809 using Firefox 63.0.1.

	Karl Dubost💡 :karlcow
	Comment 18 • 6 years ago

Maybe https://webcompat.com/issues/20527 is an instance of this issue.

	pringo4
	Comment 19 • 6 years ago

I can confirm this bug still exists in today's re-release of Windows 10 1809.

	Mathias Humpeler
	Comment 21 • 6 years ago

I cnfirm this too, same on my machine on Windows 1809, second final Version :)

	Julien Cristau [:jcristau]
	Comment 22 • 6 years ago

Too late for 64.  Adam, have you had a chance to get back to this?

	Adam Gashlin (he/him) [:agashlin] (ex-moco) Assignee
	Comment 23 • 6 years ago

It turns out that Chrome *is* doing something explicit [1] which fixes this, thought it wasn't put there with this glitch in mind: When we're supposed to be full screen, override the position given in WM_WINDOWPOSCHANGING. This way the broken position update just gets ignored. This is kind of cheating, but, without Microsoft fixing the DWM, the other approach (see hack below) is no better.

Patch incoming.

[1] https://cs.chromium.org/chromium/src/ui/views/win/hwnd_message_handler.cc?l=2667&rcl=f1b12ddf4c132dd2e846b0ee2d08e23b83581bcb

---

Another approach:

Top level widget\windows\nsWindow.cpp
+DWORD WINAPI
+ResizeThread(LPVOID lpParam)
+{
+  LPWINDOWPOS pos = static_cast<LPWINDOWPOS>(lpParam);
+  return SetWindowPos( pos->hwnd, pos->hwndInsertAfter,
+      pos->x, pos->y, pos->cx, pos->cy,
+      pos->flags);
+}

In nsWindow::Resize(double aX, double aY, double aWidth, double aHeight, bool aRepaint)
-    VERIFY(::SetWindowPos(mWnd, nullptr, x, y,
-                          width, GetHeight(height), flags));
+    WINDOWPOS resizeParam = {mWnd, nullptr, x, y, width, GetHeight(height),
+                             flags | SWP_ASYNCWINDOWPOS};
+    HANDLE hResizeThread = CreateThread(nullptr, 0, ResizeThread, &resizeParam, 0, 0);
+    WaitForSingleObject(hResizeThread, INFINITE);

This has the effect of forcing SetWindowPos to post its message (because we're calling it with SWP_ASYNCWINDOWPOS on the new thread) rather than sending it on the same thread. I guess the delay in processing makes the resize happen later, after the window for the bug.

This is also less desirable as we wouldn't want to do this on *every* size change, so some extra machinery would be needed, and we'd need to clean up the thread and pass along the return code of SetWindowPos.

	Pulsebot
	Comment 25 • 6 years ago

Pushed by agashlin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bcc079816150
Force fullscreen window position. r=jmathies

	Cristina Coroiu [:ccoroiu]
	Comment 26 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/bcc079816150

	Ryan VanderMeulen [:RyanVM]
	Comment 27 • 6 years ago

We should probably take this on ESR60 as well, but it can wait for the next cycle after it's had some bake time.

	Krzysztof
	Comment 28 • 5 years ago

I'm afraid that this bug has broken the fullscreen mode on win7. See Bug 1513066.