Closed Bug 1576391 Opened 4 months ago Closed 4 months ago

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

Categories

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

All
macOS
task

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: mstange, Assigned: mstange)

References

Details

Attachments

(3 files)

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.

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.

Blocks: 1576483
No longer depends on: 1576390
Summary: Remove override of -[NSThemeFrame _wantsFloatingTitlebar] → Remove override of -[NSThemeFrame _wantsFloatingTitlebar] when using CoreAnimation

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

No longer depends on: 1576387, 1576388
Summary: Remove override of -[NSThemeFrame _wantsFloatingTitlebar] when using CoreAnimation → Remove override of -[NSThemeFrame _wantsFloatingTitlebar] and -[NSWindow setContentView:] when using CoreAnimation

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.

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.

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

Duplicate of this bug: 1576388
Duplicate of this bug: 1576387
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Pushed by mstange@themasta.com:
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

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.

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