Dragging window to secondary screen with hwa disabled creates twin window with different scaling

VERIFIED FIXED in Firefox 46

Status

()

defect
--
critical
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: ashughes, Assigned: jfkthame)

Tracking

({regression})

45 Branch
mozilla48
Unspecified
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45- wontfix, firefox46 verified, firefox47 verified, firefox48 verified, thunderbird_esr4546+ fixed)

Details

(Whiteboard: gfx-noted)

Attachments

(2 attachments)

1. Start Firefox with a new profile
2. Disable hardware acceleration and restart Firefox
3. Drag window to a secondary screen

Result: https://youtu.be/KP2jyxwrY0Y

mozilla-central
===============
Last good revision: 9ddf0da90fb3bc1ae29966dc596013fc54a44bd2 (2015-12-29)
First bad revision: c690c50b2b543b420803e8192d6e08e06b20e0a3 (2015-12-30)
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9ddf0da90fb3bc1ae29966dc596013fc54a44bd2&tochange=c690c50b2b543b420803e8192d6e08e06b20e0a3

mozilla-inbound
===============
Last good revision: fbe047a4fbf493a602d30ff0fb903b59ab384d09
First bad revision: c0b3b26d05cfdcb53d9877a6f9388f7d3d2e20d6
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fbe047a4fbf493a602d30ff0fb903b59ab384d09&tochange=c0b3b26d05cfdcb53d9877a6f9388f7d3d2e20d6

Blame:
Markus Stange — Bug 1187322 - Don't require accelerated OpenGL contexts for BasicCompositor on OS X. r=jrmuizel
[Tracking Requested - why for this release]: regression causes inability to use Firefox on a secondary screen when hardware acceleration is disabled on Mac OS X.
Severity: normal → critical
Flags: needinfo?(mstange)
Keywords: regression
Version: Trunk → 46 Branch
Whiteboard: gfx-noted
Version: 46 Branch → 45 Branch
Does resizing the window fix it?
(In reply to Markus Stange [:mstange] from comment #2)
> Does resizing the window fix it?

Yes.
Does the tryserver build from https://treeherder.mozilla.org/#/jobs?repo=try&revision=2e49e0b8585f (see bug 1240533 comment 16) fix this?
Flags: needinfo?(anthony.s.hughes)
(In reply to Jonathan Kew (:jfkthame) from comment #4)
> Does the tryserver build from
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=2e49e0b8585f

No, it still reproduces.
Flags: needinfo?(anthony.s.hughes)
One problem is that the mBounds field of the nsChildView stays at the old value until the window is resized. mBounds is in LayoutDeviceIntPixels, so it definitely needs to be updated at some point during the resolution change.
Flags: needinfo?(mstange)
Jonathan, is this something you could look into?
Flags: needinfo?(jfkthame)
Yes, I'm trying to work through various DPI-related issues; hope to dig into this next week. (Though contributions are very welcome if anyone else has time to debug it in the meantime!)
Flags: needinfo?(jfkthame)
Great, thanks!
any update?
Flags: needinfo?(jfkthame)
I experimented a bit with this, in particular by addressing the nsChildView's mBounds issue Markus noted in comment 6; this fixes the behavior nicely in some cases, but unfortunately not all; there's still a scenario where the window ends up badly broken.
Flags: needinfo?(jfkthame)
OK, this does fix the problem with the nsChildView bounds. The remaining issue I was seeing here turns out to be bug 1256576, for which I have just posted a separate patch.
Attachment #8733792 - Flags: review?(mstange)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Attachment #8733792 - Flags: review?(mstange) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e4c92961007c882bca44d563db3afb0fb2b2b82
Bug 1248675 - Update the cached mBounds in nsChildView when its backing scale factor (display DPI) changes. r=mstange
Comment on attachment 8733792 [details] [diff] [review]
Update the cached mBounds in nsChildView when its backing scale factor (display DPI) changes

Approval Request Comment
[Feature/regressing bug #]: bug 1187322 (I think the bug here predates that, but the change in 1187322 exposed it as a user-visible issue)

[User impact if declined]: incorrectly drawn window after dragging between hi- and lo-dpi screens when HWA disabled on OS X

[Describe test coverage new/current, TreeHerder]: tested manually (involves multiple monitors & mixed DPI)

[Risks and why]: minimal; trivial fix to update widget bounds

[String/UUID change made/needed]: none

Note that on aurora (47), this is dependent on the fix for bug 1256576 (also nominated for uplift); on beta (46), that issue is not present so the patch here should be sufficient.
Attachment #8733792 - Flags: approval-mozilla-beta?
Attachment #8733792 - Flags: approval-mozilla-aurora?
Comment on attachment 8733792 [details] [diff] [review]
Update the cached mBounds in nsChildView when its backing scale factor (display DPI) changes

Support for mixed DPI screens for OS X.
For aurora, please uplift 1256576 first.
For beta this should be OK to uplift without anything special
Attachment #8733792 - Flags: approval-mozilla-beta?
Attachment #8733792 - Flags: approval-mozilla-beta+
Attachment #8733792 - Flags: approval-mozilla-aurora?
Attachment #8733792 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/0e4c92961007
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
It has been suggested to uplift this to the Thunderbird branch of m-esr45 to resolve bug Thunderbird bug 1266354.
It seems this does not build for esr45
nsChildView.mm:987:11: error: no viable overloaded '='
Right, backed out http://hg.mozilla.org/releases/mozilla-esr45/rev/999f56e779ad

Jonathan, we would still like to land this on the Thunderbird branch of mozilla-esr45 so we can fix this issue. I have no experience with the Objective-C code here, but this is only a two line change. Is there an obvious way to port this to the mozilla-esr45?
Flags: needinfo?(jfkthame)
Here's the patch ported to esr45. The widget's mBounds field wasn't using strongly-typed units in mozilla-45, so we need to convert from LayoutDevice pixels to "unknown" units for the assignment.
Flags: needinfo?(jfkthame)
Thanks, Jonathan!
Comment on attachment 8749985 [details] [diff] [review]
Update the cached mBounds in nsChildView when its backing scale factor (display DPI) changes

Pushed to THUNDERBIRD_45_VERBRANCH https://hg.mozilla.org/releases/mozilla-esr45/rev/04ca47a6ad05
Flags: qe-verify+
Reproduced the initial issue using old Nightly 2016-02-16 on a Macbook Pro + second monitor. Verified that the issue is fixed using Firefox 46.0.1 RC, 47beta9, latest Developer Edition and latest Nightly on the same hardware.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.