Last Comment Bug 814434 - [hidpi/retina] url suggestion box pops up on wrong screen
: [hidpi/retina] url suggestion box pops up on wrong screen
Status: VERIFIED FIXED
: relnote
Product: Core
Classification: Components
Component: Widget: Cocoa (show other bugs)
: 18 Branch
: x86 Mac OS X
: -- normal with 1 vote (vote)
: mozilla20
Assigned To: Jonathan Kew (:jfkthame)
: juan becerra [:juanb]
Mentors:
: 813962 817132 826402 828148 831017 (view as bug list)
Depends on: 819725 840881 858442
Blocks: osx-hidpi 794038
  Show dependency treegraph
 
Reported: 2012-11-22 08:08 PST by chrille_b
Modified: 2013-04-05 23:18 PDT (History)
22 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
verified disabled
-
verified disabled
verified


Attachments
hidpi screenshot (2.48 MB, image/png)
2012-11-22 08:29 PST, chrille_b
no flags Details
non-hidpi screen screenshot (78.03 KB, image/png)
2012-11-22 08:30 PST, chrille_b
no flags Details
always use global display pixels for popup window positioning/sizing (25.98 KB, patch)
2012-11-30 13:50 PST, Jonathan Kew (:jfkthame)
no flags Details | Diff | Splinter Review
Screenshot of clipped overlay (15.42 KB, image/png)
2012-12-03 08:50 PST, Glen Mailer
no flags Details
Screenshot of clipped overlay inside page (252.48 KB, image/png)
2012-12-03 08:50 PST, Glen Mailer
no flags Details
use global display pixels for window positioning/sizing for consistency across mixed-resolution screens (12.11 KB, patch)
2012-12-06 01:23 PST, Jonathan Kew (:jfkthame)
smichaud: review+
Details | Diff | Splinter Review
rollup for mozilla-beta (72.76 KB, patch)
2013-01-12 02:53 PST, Jonathan Kew (:jfkthame)
akeybl: approval‑mozilla‑beta-
Details | Diff | Splinter Review
pref off hidpi support for multi-display configurations (1.21 KB, patch)
2013-01-13 12:21 PST, Jonathan Kew (:jfkthame)
smichaud: review+
akeybl: approval‑mozilla‑beta+
bajaj.bhavana: approval‑mozilla‑release+
Details | Diff | Splinter Review

Description chrille_b 2012-11-22 08:08:41 PST
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:18.0) Gecko/18.0 Firefox/18.0
Build ID: 20121119042013

Steps to reproduce:

I've run FF 18+ on OS X 10.8.2 with 2 screens. One screen runs in HIDPI mode, the other runs on non HIDPI mode. If i've put the FF window to non HIDPI screen. When entering a url like 'mac' the suggestion box opens on the HIDPI screen.

The HIDPI screen is left of the non HIDPI screen. 
Notice this happens on FF versions 18(aurora)-20(nightly). 
This bug occurs also on forms (e.g. <select> with <option> tags).


Actual results:

The suggestion box pops up on the wrong screen (hidpi screen).
Look at the attached file from the hidpi screen.

It seems like FF thinks all screen are in hidpi mode and calculates the wrong relative position.


Expected results:

The suggestion box should be opened on the screen where the window is.
Comment 1 chrille_b 2012-11-22 08:29:27 PST
Created attachment 684433 [details]
hidpi screenshot
Comment 2 chrille_b 2012-11-22 08:30:33 PST
Created attachment 684434 [details]
non-hidpi screen screenshot
Comment 3 chrille_b 2012-11-25 07:27:26 PST
update: to reproduce this bug the non hidpi screen must be right of hidpi screen (primary screen). 
If non hidpi screen is left of hidpi screen (primary), boxes pop up on the expected position.
Comment 4 Glen Mailer 2012-11-27 01:47:52 PST
*** Bug 813962 has been marked as a duplicate of this bug. ***
Comment 5 Glen Mailer 2012-11-27 01:50:57 PST
Confirming bug, I'm seeing this with the setup described on the current Aurora channel.
Comment 6 Jonathan Kew (:jfkthame) 2012-11-30 13:50:06 PST
Created attachment 687273 [details] [diff] [review]
always use global display pixels for popup window positioning/sizing

WIP - patch to fix the position/size issues with various popup windows on a lo-dpi screen to the right of the Retina display.
Comment 7 Jonathan Kew (:jfkthame) 2012-11-30 13:51:14 PST
There's a test build with this patch at:

http://ftp-scl3.mozilla.com/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-adce2f5c6899/try-macosx64/firefox-20.0a1.en-US.mac.dmg

chrille_b, Glen: if you're able to test this and confirm whether it resolves the problems you're seeing, that would be great. The patch wants some more tidying up before it's ready for review/landing, but I believe it should address the basic issue.
Comment 8 chrille_b 2012-12-02 08:55:03 PST
I can confirm that your test build resolves the problems of this bug. Thank you for fixing this bug.
Comment 9 Stefan [:stefanh] 2012-12-02 14:50:24 PST
*** Bug 817132 has been marked as a duplicate of this bug. ***
Comment 10 Glen Mailer 2012-12-03 03:12:53 PST
I can also confirm that the patch above resolves the issue.
Comment 11 Glen Mailer 2012-12-03 04:47:18 PST
Sorry, I'll have to contradict myself now.

I'm still seeing some issues with some of the overlays, sometimes form autocomplete suggestions are clipped to a single item, and the search box suggestions list also seems to render with a box only tall enough to display the top item.
Comment 12 Jonathan Kew (:jfkthame) 2012-12-03 06:08:36 PST
Could you attach screenshots, and give specific steps to reliably reproduce those problems? Thanks.
Comment 13 Glen Mailer 2012-12-03 08:50:18 PST
Created attachment 687810 [details]
Screenshot of clipped overlay

Hopefully this isn't just me, I'm not sure if all of these steps are required, but they seem to work for me. The clipped overlay appears on either screen.

Cmd+N (new window)
Visit https://developer.mozilla.org/en-US/
Focus search box in UI
Press down
Overlay renders correctly
Focus search box in website
Press down
Overlay renders correctly
Enter a term in the search box which isn't already in the list
Press enter (submitting the search)
Focus the search box in the website
Clear the contents of the search box
Press down
*Overlay is clipped*
Focus the search box in the UI
Press down
*Overlay is clipped*
Comment 14 Glen Mailer 2012-12-03 08:50:48 PST
Created attachment 687811 [details]
Screenshot of clipped overlay inside page
Comment 15 Jonathan Kew (:jfkthame) 2012-12-06 01:20:10 PST
The problem with the positioning of various kinds of "pop-up" windows arises because the Move and Resize methods for the window (implemented in nsCocoaWindow) take coordinates in device pixels. This is a problem in the specific case where there's a lo-dpi screen to the right of a hi-dpi one, because in this situation, the coordinate spaces of the two screens will "overlap" (because of the different scaling they use) - e.g. the MacBook's built-in Retina screen has x-coordinates from 0 to 2880 (in "retina device pixel" that are twice what the "cocoa points" would be), while the external display has x-coordinates that start at 1440 (because they're in lo-dpi  device pixels that correspond directly to cocoa points).

As a result, when code tries to place a popup window 500px from the left of the external display, for example, this is passed as a device-pixel coordinate of 1940 ... but that is also a device-pixel coordinate within the internal retina screen, and that's where the window ends up. :(

In the initial hidpi bugs, we fixed this for window creation, so that when we persist and restore window positions they reappear in the right place, but we need to do the same for the methods that move windows around on the (global) desktop space, as screen-specific device pixels may not give us a unified, consistent coordinate space for this.
Comment 16 Jonathan Kew (:jfkthame) 2012-12-06 01:23:13 PST
Created attachment 689123 [details] [diff] [review]
use global display pixels for window positioning/sizing for consistency across mixed-resolution screens

This switches us to use display pixels for window move/resize operations, to avoid the ambiguities of device pixels. It should make no difference on non-Mac systems where there's no distinction between display and device pixels. Tryserver job at https://tbpl.mozilla.org/?tree=Try&rev=17faea330af6.
Comment 17 Jonathan Kew (:jfkthame) 2012-12-06 02:05:45 PST
Glen, could you please check whether the problem described in comment 13 is still reproducible with the latest tryserver build? (http://ftp-scl3.mozilla.com/pub/mozilla.org/firefox/try-builds/jkew@mozilla.com-17faea330af6/try-macosx64/) Thanks!
Comment 18 Steven Michaud [:smichaud] (Retired) 2012-12-06 08:37:52 PST
Comment on attachment 689123 [details] [diff] [review]
use global display pixels for window positioning/sizing for consistency across mixed-resolution screens

Looks fine to me.  (I haven't tested it, though.)

Good catch!
Comment 20 chrille_b 2012-12-06 17:54:25 PST
Sorry, I can't commit that Glens issue is fixed. This issue is fixed for me, that the suggestion box pops up on wrong screen (now it is displayed on the correct screen!). But the suggestion box for forms, that Glen reported isn't fixed. As far I can see or as far I'm able to reproduce Glens problem, this isn't fixed. 

The suggestion box on forms is not large enough. It displays only one row. It should display five or seven rows (I don't know). Sometimes the suggestion box does not appear or is only one row large.

Also the same occurs sometimes on the URL/address suggestion box, but I can't reproduce it now.
Comment 21 chrille_b 2012-12-06 18:06:09 PST
May be Glen should create a new bug, because this is a new or another issue ... and this can occur (really ???) also on a single screen configuration.
Comment 22 Jonathan Kew (:jfkthame) 2012-12-07 00:43:05 PST
Yes, please file that as a separate issue; I don't think it is directly related to the positioning problem here.

(I suspect it may be another manifestation of the issue that was filed but then closed in bug 816986. But until we have a clearer understanding, that's only a guess.)
Comment 23 Ed Morley [:emorley] 2012-12-07 06:34:59 PST
https://hg.mozilla.org/mozilla-central/rev/ac48e5c365e2
Comment 24 Glen Mailer 2012-12-07 07:44:57 PST
I'm not quite sure this can be marked as resolved, although I'll hold off reopening for now, in my use it feels like a few overlay-related bugs have been introduced, but these may not necessarily be directly related.

Can you confirm which nightly this landed in so I can try out the day before's to reproduce the other overlay issues as reported above?

In general use today on the build linked above i've been seeing a number of quirks with the overlays - including a clipped tooltip, and the awesomebar not showing any suggestions at all.

I'll try and get some proper reproducible scenarios when I can.
Comment 25 Jonathan Kew (:jfkthame) 2012-12-07 08:22:25 PST
The patch here has just merged to mozilla-central today, so the first Nightly build with it will be tomorrow's (2012-12-08).

Unless you still see the specific problem originally described here - the URL suggestions or other "popup" windows being displayed on the wrong screen when using multiple displays - please file separate reports for any remaining/new issues. That helps greatly with keeping track.

(If there are new problems that appear to be introduced by this fix, you could mark them as blocking this bug; otherwise, if they're specific to HiDPI systems but not specifically related to this change, mark them as blocking bug 785330, the general HiDPI-support tracker. But only reopen -this- bug if its specific issue is not actually fixed. Thanks a bunch!)
Comment 26 Jonathan Kew (:jfkthame) 2012-12-11 04:49:42 PST
FTR, the problem of drop-downs (sometimes?) only showing a single item has been filed as bug 820327. Any further input/discussion/suggestions there would be appreciated, as I can't currently reproduce the problem myself for investigation.
Comment 27 Jonathan Kew (:jfkthame) 2013-01-04 12:01:29 PST
*** Bug 826402 has been marked as a duplicate of this bug. ***
Comment 28 Jet Villegas (:jet) 2013-01-10 13:02:07 PST
tracking19:?

Release Drivers: we're getting multiple reports from FF18 users about this bug (and dependent bugs) now fixed in FF20. Please consider taking an uplift to FF19.
Comment 29 Jonathan Kew (:jfkthame) 2013-01-12 02:53:35 PST
Created attachment 701446 [details] [diff] [review]
rollup for mozilla-beta

Rollup patch for beta of this bug plus its dependencies (819725, 821454, 821679, 822307). The changesets all transplanted cleanly from current Aurora to Beta trees.
Comment 30 Jonathan Kew (:jfkthame) 2013-01-12 03:12:26 PST
Comment on attachment 701446 [details] [diff] [review]
rollup for mozilla-beta

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 794038

User impact if declined:
Rendering/usability glitches (especially with drop-down/popup menus) on retina Mac systems with multiple monitors; see also descriptions in bug 828514.

Testing completed (on m-c, etc.):
On Nightly for several weeks, now also on Aurora.

Risk to taking this patch (and alternatives if risky):
The initial patch here carried significant risk (which is why we didn't immediately seek to uplift it); and sure enough, we had regressions on Windows regarding context menus (bug 819725), plugins (bug 821454) and tab thumbnails (bug 821679). These are all fixed on Nightly/Aurora, and I now believe the complete set of patches is tested and stable enough that we should take them for FF19 so as to get this fix out to affected users.

An alternative would be to disable HiDPI support on multi-monitor configurations (effectively reverting to FF17 behavior, where users with an external display get scaled low-DPI (fuzzy) rendering. We could do this with a trivial pref change (set gfx.hidpi.enabled=1 as default), but losing Retina-quality rendering altogether on such systems would also be a poor user experience.

String or UUID changes made by this patch: None.
Comment 31 Alex Keybl [:akeybl] 2013-01-13 11:53:16 PST
(In reply to Jonathan Kew (:jfkthame) from comment #30)
> An alternative would be to disable HiDPI support on multi-monitor
> configurations (effectively reverting to FF17 behavior, where users with an
> external display get scaled low-DPI (fuzzy) rendering. We could do this with
> a trivial pref change (set gfx.hidpi.enabled=1 as default), but losing
> Retina-quality rendering altogether on such systems would also be a poor
> user experience.

The risk of regression for non-Mac non-HiDPI non-multi-monitor (>99.9% of our users) is much too large to take this amount of code change on Beta. This needs more bake time. Your alternative solution is a much better solution, and one that we may want to consider for an 18.0.1. Can you prepare this trivial patch for Monday?

We'll make sure to release note the behavior.
Comment 32 Jonathan Kew (:jfkthame) 2013-01-13 12:21:16 PST
Created attachment 701603 [details] [diff] [review]
pref off hidpi support for multi-display configurations

This pref change will disable HiDPI support when there are mixed hi- and lo-dpi screens. (Note that users who dynamically change their display configuration while Firefox is open - e.g. by attaching an external monitor to a retina macbook - will need to quit and restart the browser, otherwise they'll still be liable to experience problems, as we can't dynamically turn off HiDPI in an already-existing window.)
Comment 33 Jonathan Kew (:jfkthame) 2013-01-13 12:26:13 PST
(In reply to Alex Keybl [:akeybl] from comment #31)
> (In reply to Jonathan Kew (:jfkthame) from comment #30)
> > An alternative would be to disable HiDPI support on multi-monitor
> > configurations (effectively reverting to FF17 behavior, where users with an
> > external display get scaled low-DPI (fuzzy) rendering. We could do this with
> > a trivial pref change (set gfx.hidpi.enabled=1 as default), but losing
> > Retina-quality rendering altogether on such systems would also be a poor
> > user experience.
> 
> The risk of regression for non-Mac non-HiDPI non-multi-monitor (>99.9% of
> our users) is much too large to take this amount of code change on Beta.
> This needs more bake time. Your alternative solution is a much better
> solution, and one that we may want to consider for an 18.0.1. Can you
> prepare this trivial patch for Monday?

Patch posted, see above. If there's going to be an 18.0.1, I agree that we should consider backing off the HiDPI pref to 1, so as to avoid the multi-screen issues (modulo the dynamic-changes issue noted above).

Might you reconsider uplifting the "real" fix to 19 after it's had a couple more weeks bake time on Aurora, or would it be pointless to request that?
Comment 34 Alex Keybl [:akeybl] 2013-01-13 14:32:13 PST
(In reply to Jonathan Kew (:jfkthame) from comment #33)
> Patch posted, see above. If there's going to be an 18.0.1, I agree that we
> should consider backing off the HiDPI pref to 1, so as to avoid the
> multi-screen issues (modulo the dynamic-changes issue noted above).
> 
> Might you reconsider uplifting the "real" fix to 19 after it's had a couple
> more weeks bake time on Aurora, or would it be pointless to request that?

Seems too risky to take for FF19. Does attachment 701603 [details] [diff] [review] need review for uplift to 19 (and possible 18)?
Comment 35 Jonathan Kew (:jfkthame) 2013-01-13 15:05:01 PST
Comment on attachment 701603 [details] [diff] [review]
pref off hidpi support for multi-display configurations

Steven: see preceding comments; this is intended for FF19 and possibly 18, not for trunk or aurora.
Comment 36 Alex Keybl [:akeybl] 2013-01-14 12:41:25 PST
Comment on attachment 701603 [details] [diff] [review]
pref off hidpi support for multi-display configurations

Approving for Beta given our desire to uplift to FF18.0.1, and the fact that this fix has been deemed trivial. Also adding qawanted/verifyme so that QA makes sure to double check this behavior.

Jonathan - given the fact that this will be released mostly blindly, please help with testing as well (perhaps through a try build).
Comment 37 Jonathan Kew (:jfkthame) 2013-01-14 12:57:19 PST
Pushed to mozilla-beta:
https://hg.mozilla.org/releases/mozilla-beta/rev/cb8fa413605f

Tryserver job in progress (not expecting any test issues there, as tryserver doesn't test HiDPI systems anyway, but this should provide a build that QA can use):
https://tbpl.mozilla.org/?tree=Try&rev=8c189a16f563
Comment 38 bhavana bajaj [:bajaj] 2013-01-14 14:51:27 PST
(In reply to Jonathan Kew (:jfkthame) from comment #37)
> Pushed to mozilla-beta:
> https://hg.mozilla.org/releases/mozilla-beta/rev/cb8fa413605f
> 
> Tryserver job in progress (not expecting any test issues there, as tryserver
> doesn't test HiDPI systems anyway, but this should provide a build that QA
> can use):
> https://tbpl.mozilla.org/?tree=Try&rev=8c189a16f563

Can you please help nominate the patch for approval-mozilla-release in preparation to get this ready for a possible 18.0.1 ? Thanks
Comment 39 Jonathan Kew (:jfkthame) 2013-01-15 00:35:13 PST
Comment on attachment 701603 [details] [diff] [review]
pref off hidpi support for multi-display configurations

[Approval Request Comment]
Regression caused by (bug #): 794038

User impact if declined:
Rendering/usability glitches (especially with drop-down/popup menus) on retina Mac systems with multiple monitors; see also descriptions in bug 828514.

Testing completed (on m-c, etc.):
Patch itself is minimally tested, but trivial. Affected users have confirmed that disabling HiDPI support resolves the issues; this patch just changes the default value of the HiDPI pref.

Risk to taking this patch (and alternatives if risky):
This pref-change -will- "regress" behavior in the sense that Retina MacBook users with external displays will no longer get HiDPI rendering on the Retina display (until FF20, when the main patch here and its dependencies ship). This is unfortunate, but overall it's still a better experience than the problems with "popup" elements like the awesomebar suggestions appearing incorrectly across multiple displays.
Comment 40 bhavana bajaj [:bajaj] 2013-01-15 11:51:14 PST
Comment on attachment 701603 [details] [diff] [review]
pref off hidpi support for multi-display configurations

Please go ahead and land the patch on mozilla-release .
Comment 41 Jonathan Kew (:jfkthame) 2013-01-15 12:48:06 PST
https://hg.mozilla.org/releases/mozilla-release/rev/69cc79a6a535
Comment 42 Jonathan Kew (:jfkthame) 2013-01-15 12:49:00 PST
Oops, my bad - that was pushed with a=akeybl instead of a=bajaj in the commit message.
Comment 43 Matthias Versen [:Matti] 2013-01-15 16:10:35 PST
*** Bug 831017 has been marked as a duplicate of this bug. ***
Comment 44 Ioana (away) 2013-01-17 08:51:56 PST
chrille_b, can you please verify that this issue doesn't reproduce on Firefox 18.0.1 and 19 beta 2?
Comment 45 chrille_b 2013-01-17 10:00:27 PST
This bug is not reproduceable on Firefox 19b2 downloaded from here http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/19.0b2/mac/en-US/Firefox%2019.0b2.dmg

I cannot say if it is reproduceable on 18.0.1 because I don't know where to download it.
Comment 46 Matt Wobensmith [:mwobensmith][:matt:] 2013-01-17 10:23:19 PST
As per steps above, I verified URL suggestion box, Google search field, forms, select/option menus. In affected build, all were drawn on the Retina display, but drawn correctly in 19.0b2.

Also verified the pref change for gfx.hidpi.enabled, which makes the above possible.

Confirmed issue present in 19.0b1, fixed in 19.0b2.
Comment 47 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-01-17 10:53:35 PST
Thanks Matt, can you please confirm:
* Firefox 19.0b2 pref("gfx.hidpi.enabled", 1); and this bug does not exist
* Firefox 20.0a2 pref("gfx.hidpi.enabled", 2); and this bug does not exist

Thanks
Comment 48 Matt Wobensmith [:mwobensmith][:matt:] 2013-01-17 11:08:41 PST
Pref in 19.0b2 is set to 1, and issue does not exist.
Pref in 20.0a2 is set to 2, and issue does not exist.

Additionally, pref is set to 1 in 18.0 2013-01-16, and issue does not exist there as well, though it does exist in currently shipping 18.
Comment 49 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-01-17 12:10:24 PST
Excellent! Thank you Matt.
Comment 50 Sendu Bala 2013-01-19 00:07:48 PST
This is not really fixed. See bug 832591.
Comment 51 Andrew Curtin 2013-01-23 15:28:07 PST
this is a terrible fix. Sure, it fixed the pop ins (and intermittent issue), but it completely disabled retina support, and makes firefox look like garbage (IMO it's unusable) all the time (a constant issue).
Comment 52 Alex Keybl [:akeybl] 2013-01-25 14:24:54 PST
(In reply to Sendu Bala from comment #50)
> This is not really fixed. See bug 832591.

It will be fully fixed as of FF20, released the first week of April. For now, the behavior in 18.0.1 for multi-monitor setups will remain until the final fix has had more bake time with pre-release audiences.
Comment 53 Matthias Versen [:Matti] 2013-02-16 07:49:39 PST
*** Bug 828148 has been marked as a duplicate of this bug. ***

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