1576391 - Remove override of -[NSThemeFrame _wantsFloatingTitlebar] and -[NSWindow setContentView:] when using CoreAnimation

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, task, P3)

Description

[Markus Stange [:mstange]]

We added this override in bug 1400057 to avoid titlebar glitches. It's a hack and we should try to remove it again in order to reduce our chances of breakage when Apple makes changes to NSThemeFrame in the future.

In order to do this, we need to remove the setContents override and find a solution for clipped window buttons.

Here's the description of why the override is needed at the moment:

    // Override the _wantsFloatingTitlebar method to return NO, because it works around multiple
    // problems:
    // When we're not using CoreAnimation, the "floating titlebar" overlaps in a glitchy way with
    // the NSOpenGLContext when we're drawing in the titlebar. These glitches do not happen when we
    // use CoreAnimation.
    // An additional problem appears in builds that link against the 10.14 SDK: In windows where
    // _wantsFloatingTitlebar returns YES, the root NSView contains an additional view that provides
    // a window background. This confuses our setContentView override which will place the content
    // view *below* that background view, effectively hiding the content view completely.
    // The floating titlebar view also slightly clips the bottom of the window buttons which we
    // forcefully move down with our override of _closeButtonOrigin.

Comment 1

[Markus Stange [:mstange]]

It looks like this override prevents window buttons from rendering on 10.13 (see bug 1576483), so we have to fix this as soon as possible.

Comment 2

[Markus Stange [:mstange]]

I'm combining all three bugs into this one because they have to land together.

Comment 3

[Markus Stange [:mstange]]

Attachment: Bug 1576391 - Move some method overrides to ToolbarWindow. r=spohl

This makes them only apply in windows with titlebars, which is the only place
where they're needed. The setContentView override can even cause harm for other
windows, such as sheet windows, because it'll move the content view below a
full-window covering solid grey view provided by the system, in builds that
link against the 10.14 SDK and don't override _wantsFloatingTitlebar.

Comment 4

[Markus Stange [:mstange]]

Attachment: Bug 1576391 - Use NSFullSizeContentViewWindowMask for ToolbarWindows. r=spohl

This gives us two behaviors for free which we were achieving through manual
overrides:

• The content view is sized to cover the entire window frame.
• The window controls are placed on top of the content view (instead of
  underneath it in z-order).

It also forces CoreAnimation layers for the window's entire NSView hierarchy, so
we only use it when the CoreAnimation pref is enabled.

NSFullSizeContentViewWindowMask is only available on 10.10 and up, but we still
support 10.9, so we cannot remove the code with the manual overrides just yet.

This change also requires a change to NonDraggableView in order to preserve
window dragging behavior in the titlebar: When NSFullSizeContentViewWindowMask
is used, the method which assembles the window's draggable region takes a
different path. It treats the titlebar specially, and traverses the NSView
hierarchy twice, once for the titlebar area and once for the rest of the window.
Outside the titlebar, it calls _opaqueRect on every visible NSView, but for the
titlebar area, it calls _opaqueRectForWindowMoveWhenInTitlebar instead.

Overriding _opaqueRectForWindowMoveWhenInTitlebar allows us to achieve the old
dragging behavior.

Comment 5

[Markus Stange [:mstange]]

Attachment: Bug 1576391 - Override _titlebarHeight so that the window's NSTitlebarContainerView includes the full height of the window buttons even after we've shifted them down, and re-enable the floating titlebar. r=spohl

I've tried hard to find a solution for the clipped window button problem which
does not involve swizzling but could not find one.
The officially ratified ways of moving the window buttons are:

1. Use an NSToolbar in your window, or
2. Make a custom window with your own buttons.

I don't think having an NSToolbar is an option for us. And making our own window frame
implementation would be hard to get right and can easily behave differently from the
native implementation.
I've also tried asking the window to not render any window buttons on its own; then we
could make our own buttons and place them on top of our view. But if I change the
window's styleMask to be just NSTitledWindowMask, it doesn't only make the buttons
disappear, it also makes the window non-resizable. And if I include NSResizableWindowMask,
the buttons come back (only the zoom button is enabled, the other two are grayed out).

This change also stops overriding _wantsFloatingTitlebar when CoreAnimation is
enabled, which is necessary for the window buttons to render.

Depends on D43341

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b1ea03f73bae
Move some method overrides to ToolbarWindow. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/d09424718fea
Use NSFullSizeContentViewWindowMask for ToolbarWindows. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/70ec0ddc8f32
Override _titlebarHeight so that the window's NSTitlebarContainerView includes the full height of the window buttons even after we've shifted them down, and re-enable the floating titlebar. r=mattwoodrow

Comment 9

[Markus Stange [:mstange]]

Stephen, I asked Matt to take a look at these patches so that I could land them sooner and fix the regression in bug 1576483. But it would be great if you could give them another review once you're back. I'll needinfo you on this bug in a few days.

Comment 10

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/b1ea03f73bae
https://hg.mozilla.org/mozilla-central/rev/d09424718fea
https://hg.mozilla.org/mozilla-central/rev/70ec0ddc8f32