1934040 - Wrong titlebar buttonbox height with Mica effect enabled

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect, P3)

Description

[Ugi]

Attachment: with and without mica.png

Steps to reproduce:

1. Set "widget.windows.mica" to true in about:config (Nightly)
2. Set Firefox theme to system theme - auto
3. Restart

Actual results:

I found a comment in the buttonbox CSS that explains it very well

@media (-moz-windows-accent-color-in-titlebar) or (-moz-windows-mica) {
:root[tabsintitlebar] {
@media (-moz-windows-mica) {
&:not([lwtheme]) {
background-color: transparent;

    /* Accent color in titlebar overrides Mica. */
    @media not (-moz-windows-accent-color-in-titlebar) {
      /* Hide the Firefox buttons since Windows draws their own.
       * FIXME(emilio): Find a way of hiding the native buttons, or make them
       * taller, this is very hacky. */
      .titlebar-buttonbox {
        opacity: 0;
      }
    }
  }
}

}
}

I didn't saw any bug report about it, hopefully this is being worked on (by emilio I guess?).

Expected results:

The button box should be the same height as its parent container as before.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Audio/Video' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Yes, I know about this, and I've looked into it. But so far I haven't found a way to make it work that doesn't regress other things (like snap layouts). I'll post some more context in a bit...

Comment 3

[Emilio Cobos Álvarez [:emilio]]

In the Aero days, we basically just dealt with this by hiding our own controls and reserving that space for DWM. It seems that's what chromium is doing too. But that's not great.

The only reliable way I found to hide the DWM caption buttons is to flip the relevant WS_{SYSMENU,MINIMIZEBOX,MAXIMIZEBOX} bits in the window style, or flip WS_CAPTION entirely. But that has too many side effects:

• Maximize covers the task bar (documented quirk).
• tiling, double-click, snap layouts, etc all stop working.

I realized Edge manages to stash its own window controls on its mica titlebar, so there's probably something that can be done about this? But I haven't found it so far. I'm aware of the AppWindowTitlebar.PreferredHeightOption thing exposed in the Windows SDK, but whatever lower level mechanism that uses, which is what we'd have to use (maybe a custom DWMWindowAttribute or something?) doesn't seem to be documented.

Comment 5

[Ugi]

I don't know if it's helpful, but the settings page on Microsoft’s PowerToys, which is open source, has Mica implemented, and from what I can see, the buttonbox is tall, or at least taller than what Firefox is showing. Perhaps their code can help us understand how they are handling the issue.

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Yeah they use the .PreferredHeightOption = Tall mentioned above (here).

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Was bug 1934138 removed intentionally from the blocking list? Custom-drawing our buttons would fix it.

Comment 8

[Ugi]

No, sorry I'm not really familiar with Bugzilla. The interface is a bit overwhelming and I might have messed something up when I replied. Feel free to reverse that, I have no clue how to do it myself.

Comment 9

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1934040 - Remove WS_SYSMENU when using custom titlebar, so that DWM doesn't draw caption buttons. r=#win-reviewers!

Unhide our buttons instead, which fixes the caption button sizes when
Mica is enabled.

Crucially (and this is what I had failed to do in my original attempts),
we need to keep WS_MAXIMIZEBOX for Snap Layouts and Aero snap to keep
working. WS_MINIMIZEBOX doesn't hurt either.

This is a bit scary, because we're in undocumented territory, but afaict
(looking with winspy), MS Edge does the same for its custom titlebar, so
I think this is probably safe. It's early in the cycle anyways...

Comment 10

[Emilio Cobos Álvarez [:emilio]]

Ok, I think I found something that doesn't cause side effects...

Comment 11

[Sam Johnson]

FWIW, I noticed that the tabbed version of File Explorer also has buttons shorter than its Mica titlebar. Given that it ships with the OS and is a core app, maybe this isn't necessarily an issue and more of a preference?

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/838165f60521
Remove WS_SYSMENU when using custom titlebar, so that DWM doesn't draw caption buttons. r=win-reviewers,rkraesig

Comment 15

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/838165f60521