I updated my video drivers yesterday and after a restart now all of my desktop notifications from Firefox are appearing in the middle of my screen. window.screen on a regular webpage shows > Screen { availWidth: 1920, availHeight: 1040, width: 1920, height: 1080, colorDepth: 24, pixelDepth: 24, top: 0, left: 0, availTop: 0, availLeft: 0 } window.screen for the browser window shows > Screen { availWidth: 1920, availHeight: 1040, width: 1920, height: 1080, colorDepth: 24, pixelDepth: 24, top: 0, left: 0, availTop: 0, availLeft: 0 } and the browser correctly paints everything in the browser chrome and content to fill the window when the window is maximized. layout.css.devPixelsPerPx is set to -1.0.

My resolution of this display is 3840x2160 and it has display scaling set to 200%.

I used mozregression and tracked it down to bug 890156. Jonathan, can you take this?

6:04.67 LOG: MainThread mozversion INFO application_buildid: 20160112231330 6:04.67 LOG: MainThread mozversion INFO application_changeset: 98756a36223c1a2b51cd0368736b728429038caf 6:04.67 LOG: MainThread mozversion INFO application_display_name: Nightly 6:04.67 LOG: MainThread mozversion INFO application_id: {ec8030f7-c20a-464f-9b0e-13a3a9e97384} 6:04.67 LOG: MainThread mozversion INFO application_name: Firefox 6:04.67 LOG: MainThread mozversion INFO application_remotingname: firefox 6:04.67 LOG: MainThread mozversion INFO application_repository: https://hg.mozilla.org/integration/mozilla-inbound 6:04.67 LOG: MainThread mozversion INFO application_vendor: Mozilla 6:04.67 LOG: MainThread mozversion INFO application_version: 46.0a1 6:04.67 LOG: MainThread mozversion INFO platform_buildid: 20160112231330 6:04.67 LOG: MainThread mozversion INFO platform_changeset: 98756a36223c1a2b51cd0368736b728429038caf 6:04.67 LOG: MainThread mozversion INFO platform_repository: https://hg.mozilla.org/integration/mozilla-inbound 6:04.67 LOG: MainThread mozversion INFO platform_version: 46.0a1 Was this inbound build good, bad, or broken? (type 'good', 'bad', 'skip', 'retry', 'back' or 'exit' and press Enter): good 6:14.57 LOG: MainThread Bisector INFO Narrowed inbound regression window from [f58e7ca0, a9f9b36c] (3 revisions) to [98756a36, a9f9b36c] (2 revisions) (~1 steps left) 6:14.57 LOG: MainThread main INFO Oh noes, no (more) inbound revisions :( 6:14.57 LOG: MainThread Bisector INFO Last good revision: 98756a36223c1a2b51cd0368736b728429038caf 6:14.57 LOG: MainThread Bisector INFO First bad revision: a9f9b36c1a2eec7626e6b749e46ab0a8bf3323e2 6:14.57 LOG: MainThread Bisector INFO Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=98756a36223c1a2b51cd0368736b728429038caf&tochange=a9f9b36c1a2eec7626e6b749e46ab0a8bf3323e2

You're on Linux, right? I'd guess this is probably the same as bug 1239855. Begun investigating...

No, Windows 10 (Build 11082), as the screenshot shows ;)

It also occurs on Linux (with the difference that the notification is at the middle in the top part of the screen).

[Tracking Requested - why for this release]: Alert notifications on Windows and Linux will appear in the center or top-center of the screen, instead of the top-right corner.

I expect this to affect all platforms where we use toolkit/components/alerts/resources/content/alert.js to display these notifications (which doesn't seem to include OS X, afaict). Currently investigating and experimenting; I think I have some idea what is going wrong, and hope to have a patch before too long....

Cool; thanks, Jonathan!

Tracking for 46 since this is a recent regression. Since a duplicate, bug 1241178, was marked for e10s tracking I'm adding that flag as well.

These methods must operate with CSS pixels because they are DOM0 API and visible from content. I removed a redundant comment. The return type is self-descriptive enough. This patch will also fix this bug.

It's correct that these methods need to revert to CSS pixels (that was an erroneous approach in bug 890156, sorry!). So this patch is a start, and fixes this bug here, but unfortunately it will also regress behavior of window placement in some multi-monitor/mixed-dpi scenarios. I'm currently testing some additional patches that aim to help with this, and hope to have something usable soon.

Comment on attachment 8715269 [details] [diff] [review] Fix pixel units for screenX, screenY, and moveTo() (In reply to Jonathan Kew (:jfkthame) from comment #16) > but unfortunately it will also regress behavior of > window placement in some multi-monitor/mixed-dpi scenarios. Ah, you are right. Canceling the review for now.

I think what we need to do here is something along these lines. Will be testing a bit more locally before flagging for review, but so far this is working well for me.

Updated patch to fix a leak. Seems to work locally (on both windows and osx) with mixed-dpi displays. Try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=00876f864de7.

Comment on attachment 8715961 [details] [diff] [review] Revert to CSS-pixel units for screenX, screenY, moveTo() APIs, and adjust the origin for secondary displays with differing resolution to avoid overlapping coordinate spaces Review of attachment 8715961 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/nsIScreen.idl @@ +80,5 @@ > + * > + * This seems poorly named (something like devicePixelsPerDesktopPixel > + * would be more accurate/explicit), but given that it is exposed to > + * content as well as used internally, it's probably not worth the > + * disruption of changing it. Is this really exposed to content? The content window.screen maps to WebIDL Screen rather than XPCOM nsIScreen. I couldn't find this property on window.screen.

Comment on attachment 8715961 [details] [diff] [review] Revert to CSS-pixel units for screenX, screenY, moveTo() APIs, and adjust the origin for secondary displays with differing resolution to avoid overlapping coordinate spaces Review of attachment 8715961 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsGlobalWindow.cpp @@ +5087,5 @@ > : DesktopToLayoutDeviceScale(1.0); > + DesktopRect screenRectDesk = screenRectDev / scale; > + > + CSSPoint cssPt = > + LayoutDevicePoint(x - screenRectDesk.x, y - screenRectDesk.y) / It should use screenRectDev rather than screenRectDesk. Otherwise it will not convert the coordinates correctly at least on system DPI-aware Windows (that is, Win7 or earlier or when we revert the manifest change to disable per-monitor DPI support). I don't know why screenRectDesk works on OS X. For all other systems, it should have no difference because screenRectDesk == screenRectDev on those systems.

(In reply to Masatoshi Kimura [:emk] from comment #21) > Comment on attachment 8715961 [details] [diff] [review] > Revert to CSS-pixel units for screenX, screenY, moveTo() APIs, and adjust > the origin for secondary displays with differing resolution to avoid > overlapping coordinate spaces > > Review of attachment 8715961 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: widget/nsIScreen.idl > @@ +80,5 @@ > > + * > > + * This seems poorly named (something like devicePixelsPerDesktopPixel > > + * would be more accurate/explicit), but given that it is exposed to > > + * content as well as used internally, it's probably not worth the > > + * disruption of changing it. > > Is this really exposed to content? > The content window.screen maps to WebIDL Screen rather than XPCOM nsIScreen. > I couldn't find this property on window.screen. Ah, right - I should have said "exposed to front-end code and add-ons" (I believe).

OK, I think you're right (comment 22); adjusted patch accordingly. New try build in progress: https://treeherder.mozilla.org/#/jobs?repo=try&revision=42ca1982e50e.

Comment on attachment 8716289 [details] [diff] [review] Revert to CSS-pixel units for screenX, screenY, moveTo() APIs, and adjust the origin for secondary displays with differing resolution to avoid overlapping coordinate spaces Review of attachment 8716289 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsGlobalWindow.cpp @@ +7052,5 @@ > aError.Throw(NS_ERROR_FAILURE); > return; > } > > + LayoutDevicePoint devPos; nit: move this down to where this is first used. @@ +7072,5 @@ > + double scale; > + screen->GetDefaultCSSScaleFactor(&scale); > + devPos = cssPos * CSSToLayoutDeviceScale(scale); > + devPos.x += screenLeft; > + devPos.y += screenTop; This should also be device pixels, no?

One more try, with fix for comment 25. https://treeherder.mozilla.org/#/jobs?repo=try&revision=969eb84bacc5

Comment on attachment 8716339 [details] [diff] [review] Revert to CSS-pixel units for screenX, screenY, moveTo() APIs, and adjust the origin for secondary displays with differing resolution to avoid overlapping coordinate spaces Looks good.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7d6e0141de4fab594b48d54663a4cc90fc78e248 Bug 1240085 - Revert to CSS-pixel units for screenX, screenY, moveTo() APIs, and adjust the origin for secondary displays with differing resolution to avoid overlapping coordinate spaces. r=emk

Comment on attachment 8716339 [details] [diff] [review] Revert to CSS-pixel units for screenX, screenY, moveTo() APIs, and adjust the origin for secondary displays with differing resolution to avoid overlapping coordinate spaces >diff --git a/widget/nsIScreen.idl b/widget/nsIScreen.idl >--- a/widget/nsIScreen.idl >+++ b/widget/nsIScreen.idl >@@ -69,13 +69,30 @@ interface nsIScreen : nsISupports ... >+ readonly attribute double defaultCSSScaleFactor; Shouldn't this change come with a new UUID?

(In reply to neil@parkwaycc.co.uk from comment #30) > >+ readonly attribute double defaultCSSScaleFactor; > Shouldn't this change come with a new UUID? We have removed the UUID rev'ing requirement: https://lists.mozilla.org/pipermail/dev-platform/2016-January/013297.html

Comment on attachment 8716339 [details] [diff] [review] Revert to CSS-pixel units for screenX, screenY, moveTo() APIs, and adjust the origin for secondary displays with differing resolution to avoid overlapping coordinate spaces Approval Request Comment [Feature/regressing bug #]: Bug 890156 (mixed-DPI support for Windows) [User impact if declined]: Incorrect positioning of notifications on hidpi display. Incorrect scaling of DOM API coordinates (screenX, screenY, moveTo() method) may result in other breakage for websites as well as within Firefox or addons, as this is API that is exposed to the web. [Describe test coverage new/current, TreeHerder]: Tested manually; we don't have support for multi-screen, mixed-dpi configurations in automation. [Risks and why]: Moderate risk; there's some complexity here, but mitigated by the fact that for non-hidpi/single-monitor systems, most of the complexity drops out. [String/UUID change made/needed]: Adds a new attribute to nsIScreen, but we no longer need to rev the UUID for this. (If we don't uplift this patch, I think we should back out the full set of patches from bug 890156 and its followups on Aurora. The incorrect behavior of the DOM screen APIs should not be shipped to release channels.)

Unfortunately this seems to have caused a large regression on the tp5o scroll benchmark when e10s is enabled. :( See bug 1247049.

This is now fixed for me. Thanks!

Comment on attachment 8716339 [details] [diff] [review] Revert to CSS-pixel units for screenX, screenY, moveTo() APIs, and adjust the origin for secondary displays with differing resolution to avoid overlapping coordinate spaces Cancelling aurora approval request as bug 890156 has now been backed out of aurora-46, making this irrelevant.

This is not fixed for me, I'm now seeing notifications on the left of the screen.

I've filed bug 1248427.

Marking fixed for 46. I'll see if it makes sense to track bug 1248427 instead.