1217819 - [Control Center] Moving window to Retina display breaks doorhanger

Status: NEW

(Toolkit :: UI Widgets, defect)

Description

[Michael Kohler [:mkohler]]

Attachment: Screen Shot 2015-10-23 at 14.32.27.png

When you move a window from a non-Retina to a Retina display, it breaks the doorhanger view. It doesn't get filled all the way and cuts off text as you can see here https://usercontent.irccloud-cdn.com/file/tTsFqzTK/Screen%20Shot%202015-10-23%20at%2014.32.27.png. Reopening the doorhanger doesn't help either. Even though the screenshot is in German, this happens with Firefox 42 en_us as well.

(this is not originally reported by me, but a friend of mine)

Comment 1

[Paul Silaghi, QA [:pauly]]

Confirmed on FF 42b9, 44.0a1 (2015-10-23), OS X 10.9.5.

Comment 2

[Brian Grinstead [:bgrins]]

Hi, can you confirm the same / similar issue on other panels (like downloads popup, bookmark popup, hamburger menu)?

Comment 3

[Paul Silaghi, QA [:pauly]]

(In reply to Brian Grinstead [:bgrins] from comment #2)
> Hi, can you confirm the same / similar issue on other panels (like downloads
> popup, bookmark popup, hamburger menu)?
Only to downloads panel, from those 3.
I also see for the Hello panel, but when moving the browser window vice-versa, from retina to non-retina.

Comment 4

[Brian Grinstead [:bgrins]]

(In reply to Paul Silaghi, QA [:pauly] from comment #3)
> (In reply to Brian Grinstead [:bgrins] from comment #2)
> > Hi, can you confirm the same / similar issue on other panels (like downloads
> > popup, bookmark popup, hamburger menu)?
> Only to downloads panel, from those 3.
> I also see for the Hello panel, but when moving the browser window
> vice-versa, from retina to non-retina.

Alright, sounds like this might be an issue with panels in general.  Neil, do you know of an existing bug for this?

Comment 5

[Neil Deakin]

The popups here seem to have either a maxwidth or maxheight assigned. This will cause constraints to be assigned by calling nsIWidget::SetSizeConstraints, but this is only called again is the real size changed. One option is to remove the 'if (sizeChanged)' conditional within nsMenuPopupFrame::LayoutPopup around the call to SetSizeConstraints entirely. Might cause some other issues though.

A safer option might be check if the size has changed using device pixels instead.

I don't think there's any kind of notification or flag indicating that a reflow is happening because of a dpi change is there?

Timothy, any thoughts?

Comment 6

[Timothy Nikkel (:tnikkel)]

Hmm, perhaps the simplest solution is just to cache the AppUnitsPerDevPixel on the menupopupframe the last time LayoutPopup was called. Then if the current AppUnitsPerDevPixel is different from the last AppUnitsPerDevPixel always set the size constraints.

I don't know of a way to detect if a Reflow comes from dpi change, we could probably add one, but the above solution seems simpler unless there are other consumers that would also want the same thing.

There is nsPresContext::UIResolutionChanged, but then you have to find all the popups that might need to new size constraints.

Doing the computation in dev pixels sounds good, but to do it properly we should do the full calculation in nsContainerFrame::SetSizeConstraints. But that calls nsIWidget::ClientToWindowSize, and some window geometry calls can take significant overhead (we've had problems with gtk before, don't have the bug number handy ATM, but I don't think it was this specific widget function), and I'd be reluctant to add that to every reflow.

Comment 7

[Neil Deakin]

OK, so the above is right but the solution is more complicated.

The issue is that LayoutPopup is called to position/size the popup and set the constraints before the popup has opened. However, when the popup is opened, closed, then the parent window moved to another screen, setting the size constraints in nsCocoaWindow::SetSizeConstraints uses the old screen within BackingScaleFactor(). That is, [NSWindow backingScaleFactor] returns the old value until the popup has opened.

One option is to have BackingScaleFactor() determine or take a screen that should be used instead.

Comment 8

[Timothy Nikkel (:tnikkel)]

Is this because we haven't actually moved the widget (via Resize or Move) to the new screen yet when we call SetSizeConstraints? Can we just always call SetSizeConstraints after we show or move the popup? Or something like that.

Comment 9

[Neil Deakin]

I think the issue is that we call SetSizeConstraints before the popup is made visible, but Cocoa doesn't consider it to be on a different screen until the popup is visible. I'll investigate to see if we need to move where we set the size constraints or find if there is away to indicate beforehand which screen the popup shoule be on.