853252 - [hidpi] Fix clicking on <select> elements

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Brian Warner [:warner :bwarner]]

(not sure where this fits, or how to describe it, but I work on the 7th floor over by the SFO-7S conference room and I can show it to you)

I'm using current beta firefox (20.0) on a Mac Book Pro (retina display) with an external LCD attached. When I drag the FF window down onto the built-in retina display, then view a page like this bugzilla new-bug page, then click on one of the selection lists like "Hardware:" or "OS:" or "Status:", the list of options pops up. But when I try to click on one of those options, the browser does not respond to my click. I can click-and-drag-and-release to select one of the options, and I can click-once and then use up/down arrow and RETURN to select something, but trying to click inside the popup list does nothing.

This doesn't happen when I drag the browser window onto the external LCD: I can click on option lists correctly. It also doesn't happen with FF release (18.0.2), where the text is suspiciously fuzzy. The text is nice and hidpi-sharp on beta.

My hunch is that the HiDPI bounding boxes are off for certain windows, so the hit-detection is failing. I've seen earlier problems with this that seem to be fixed now (Persona login popup frame being the right size but its contents being drawn double-sized, also the NoScript toolbar-icon menu being clippped the second time it's drawn), so I bet it's just another variation on that theme.

thanks,
 -Brian, [e-mail]

Comment 1

[Steven Michaud [:smichaud] (Retired)]

What version of OS X are you running?

Does this also happen in today's mozilla-central nightly?

Does it make a difference where you start Firefox (on the external display or on your MBP's built-in display)?

I assume your external display is non-Retina.  Is it?

Comment 2

[Jonathan Kew [:jfkthame]]

This sounds suspiciously similar to bug 839565 - same underlying issue, maybe?

Comment 3

[Brian Warner [:warner :bwarner]]

OS-X 10.8.3

Yes, the problem exists in today's Nightly ("22.0a1 (2013-03-25)", rev 3acbf951b3b1).

External display is non-Retina (it's a Dell U2410 LCD, 1920x1200). Laptop display is configured to "Best for Retina Display", which appears to mean 2880x1800.

I don't think I can easily control where Firefox starts. I have the external monitor configured as the "primary" (menu-bar) display, and the laptop's screen is below that (and holds the Dock). Starting minefield presents the profile-chooser on the external display, and the subsequent window opens on the external display, and drop-down selection-menus work correctly while the window is on that display. When I move the window down to the laptop's HiDPI screen, the menus break. If I disconnect the external display, leaving just the laptop's screen, the menus work correctly.

If I restart minefield while there is no external display, the menus work correctly. If I then plug in the external monitor, the window jumps to the external monitor, and menus still work correctly. When I then drag the window down to the laptop screen, they break again.

Seems pretty tightly correlated with "an external monitor is attached but the window is displayed on the laptop's screen".

I've also noticed a transition occur when I drag the window partially between screens: once it gets moved far enough, the window is suddenly briefly rendered at either double-size or half-size, then corrects itself. The selection-menu changes from working to non-working or back at this transition point. When the popup list is displayed on the laptop screen (while the external monitor is attached), it breaks, and when it is displayed on the external monitor, it works.

Comment 4

[Brian Warner [:warner :bwarner]]

(In reply to Jonathan Kew (:jfkthame) from comment #2)
> This sounds suspiciously similar to bug 839565 - same underlying issue,
> maybe?

Yeah, that sounds like the same symptoms. I haven't experimented with left/right display placement, though.

Comment 5

[Brian Warner [:warner :bwarner]]

BTW, I filed bug 854477 about a popup-menu-background-compositing problem that occurs in the same circumstances, which might also be the same underlying issue.

Comment 6

[Steven Michaud [:smichaud] (Retired)]

> I have the external monitor configured as the "primary" (menu-bar)
> display, and the laptop's screen is below that (and holds the Dock).

Thanks for this info.  This is probably a somewhat unusual
arrangement, and wasn't how I was testing.

Comment 7

[Steven Michaud [:smichaud] (Retired)]

It's possible this bug is related to bug 838239, or at least can be fixed in the same way.  Here's a tryserver build made from Jonathan Kew's patch from bug 838239 comment #13:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-27436e6ebf35/try-macosx64/firefox-22.0a1.en-US.mac.dmg

Please try it out and let us know your results.

Comment 8

[Brian Warner [:warner :bwarner]]

The tryserver build is.. better, but doesn't work consistently. I seem to be able to get it into a state where the select-menu is broken like before, but sometimes it works correctly. If I put the browser window on the external display, then shrink it, then drag the window onto the laptop's hidpi screen (keeping it in the center of the screen, leaving the window at the same size), then the menus continue to work. If I then change the size of the window while it's on the hidpi screen, it seems to switch into the non-working state. And sometimes, changing the size makes it flip from non-working to working.

Are there any logs or instrumentation I should be looking at?

Comment 9

[Steven Michaud [:smichaud] (Retired)]

Thanks, Brian, for the info.  It was a real stretch that the tryserver build would make any difference, but I thought it was worth trying.

> Are there any logs or instrumentation I should be looking at?

I can't think of any that would help.

Now that we know exactly how your displays are set up, one of us needs to try again to reproduce your bug.

Comment 11

[chrille_b]

The problem is that the option element has the wrong position once you click on the select element. The bounding box for option element is calculated on the wrong pixel ratio. 

See attached screenshots. It is visible by web inspector.

Comment 12

[chrille_b]

Attachment: screenshot after clicking on select

Comment 13

[chrille_b]

Attachment: screenshot before clicking on select

Comment 14

[Timothy Nikkel (:tnikkel)]

I don't have a retina display to debug this but if I did I would start by looking at the coords in nsIFrame::GetTransformMatrix in the list control frame popup case and the coords in nsLayoutUtils::GetEventCoordinatesRelativeTo for the relevant event.

Comment 15

[Jonathan Kew [:jfkthame]]

The list control frame popup case in nsIFrame::GetTransformMatrix doesn't appear to be involved in this. When clicking around on various <select>s here, the only calls to GetTransformMatrix() I see happening look like:

[0x148c89bb0] nsIFrame::GetTransformMatrix()
   IsPopup? N GetType() = ComboboxControlFrame
[0x1128bf420] nsIFrame::GetTransformMatrix()
   IsPopup? N GetType() = ViewportFrame
[0x148cac470] nsIFrame::GetTransformMatrix()
   IsPopup? N GetType() = ComboboxControlFrame
[0x1128bf420] nsIFrame::GetTransformMatrix()
   IsPopup? N GetType() = ViewportFrame
[0x148ca9168] nsIFrame::GetTransformMatrix()
   IsPopup? N GetType() = ComboboxControlFrame
[0x1128bf420] nsIFrame::GetTransformMatrix()
   IsPopup? N GetType() = ViewportFrame
[0x148e37b88] nsIFrame::GetTransformMatrix()
   IsPopup? N GetType() = ComboboxControlFrame
[0x1128bf420] nsIFrame::GetTransformMatrix()
   IsPopup? N GetType() = ViewportFrame

Comment 16

[Jonathan Kew [:jfkthame]]

(In reply to chrille_b from comment #11)
> The problem is that the option element has the wrong position once you click
> on the select element. The bounding box for option element is calculated on
> the wrong pixel ratio. 
> 
> See attached screenshots. It is visible by web inspector.

Thanks, that looks like a useful clue.

Note that the web inspector functionality is currently broken on trunk (bug 860742); however, I can reproduce your example provided I build from an earlier tree prior to that breakage.

Comment 17

[Jonathan Kew [:jfkthame]]

nsLayoutUtils::GetEventCoordinatesRelativeTo was a good starting point for debugging this further. It turns out that in certain circumstances the nsCocoaWindow widget that's created for the <option>s in the <select> is returning 1.0 from GetDefaultScale(), even though it's being displayed on the Retina screen where this should be 2.0.

This results, naturally, in completely wrong values returned by TranslateWidgetToView(), as this depends on the window's WidgetToScreenOffset(), which depends on the backing scale factor (the value returned by GetDefaultScale()).

The cause of this is that the widget is being created with zero size at a location that implies it's on the external (non-hidpi) display, so we consider it to have a backing scale of 1.0; however, Cocoa treats all zero-size windows as having backing scale 2.0, regardless of their location. So then when the window is displayed on the Retina screen, it *doesn't* send us a notification that the backing properties have changed, and so we continue to treat it has having a scale of 1.0. Sadness.

To fix this, I propose to check within the windowDidChangeScreen delegate method - which we're already handling anyway - for a mismatch in the backing scale.

Comment 18

[Jonathan Kew [:jfkthame]]

Attachment: check for changed backing scale when we receive a windowDidChangeScreen message

Comment 19

[Jonathan Kew [:jfkthame]]

Tryserver job https://tbpl.mozilla.org/?tree=Try&rev=45a069651f43 should be available for testing in a couple of hours, if all goes well.

Comment 20

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 736454 [details] [diff] [review]
check for changed backing scale when we receive a windowDidChangeScreen message

Looks fine to me.

How bizarre!  I wonder if this might help with bug 838239 (if we might now be able to patch that bug just by calling shell->BackingScaleFactorChanged() from nsDocShell::RestoreFromHistory()).

Comment 21

[Jonathan Kew [:jfkthame]]

I'm hoping to take another look at that tomorrow, and see if there's any link.

Comment 22

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c81138a2c6ad

Comment 24

[Ryan VanderMeulen [:RyanVM]]

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

Comment 25

[Jonathan Kew [:jfkthame]]

Comment on attachment 736454 [details] [diff] [review]
check for changed backing scale when we receive a windowDidChangeScreen message

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

User impact if declined: in certain situations with mixed-resolution screens, <select> popups do not respond to clicks properly

Testing completed (on m-c, etc.): several weeks in Nightly

Risk to taking this patch (and alternatives if risky): minimal

String or IDL/UUID changes made by this patch: none

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/86c570ad3b75

Comment 28

[juan becerra [:juanb]]

Verified on Fx22b4 using Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:22.0) Gecko/20100101 Firefox/22.0 (20130605070403) using the descriptions in comment #0, comment #3 and the test case in bug 839565.

Tested that I click on select elements in hiDPI and external monitor without problems.