1401195 - The below part of the Tab Bar doesn't react on double-clicks when Dark theme is enabled

Status: VERIFIED FIXED

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

Description

[ovidiu boca[:Ovidiu]]

[Affected versions]:

FF release 55, FF beta 56, FF Nightly 57.0a1(2017-09-18)

[Affected platforms]:

Mac OS 

[Steps to reproduce]:

1. Go to Customize and enabled "Dark" or "Light" theme
2. Double-click on the Tab Bar (in the area from the middle to the bottom)

[Expected result]:

The window should expand/shrink 

[Actual result]:

Nothing happens.


[Additional notes]:

This issue is a follow-up from https://bugzilla.mozilla.org/show_bug.cgi?id=1399930#c20

Comment 1

[Dão Gottwald [:dao]]

Did this regress at some point before 55 or has this just always been broken?

Comment 2

[ovidiu boca[:Ovidiu]]

I tried with FF 50 release and I can still reproduce this.

Comment 4

[Mehmet]

It seems, that in FF59, the Light Theme is fixed. But the Dark Theme is still broken.


FF57
====
  Light: broken
   Dark: broken



FF58
====
  Light: broken
   Dark: broken



FF59
====
  Light: works
   Dark: broken

Comment 5

[Dão Gottwald [:dao]]

(In reply to Mehmet from comment #4)
> It seems, that in FF59, the Light Theme is fixed. But the Dark Theme is
> still broken.

Would be interesting to know when exactly it got fixed so we can apply the same fix to the Dark theme.

Comment 6

[Johann Hofmann [:johannh]]

(In reply to Dão Gottwald [::dao] from comment #5)
> (In reply to Mehmet from comment #4)
> > It seems, that in FF59, the Light Theme is fixed. But the Dark Theme is
> > still broken.
> 
> Would be interesting to know when exactly it got fixed so we can apply the
> same fix to the Dark theme.

Presumably in bug 1417709 which introduced -moz-mac-vibrant-titlebar-light/dark to avoid the -moz-window-titlebar hack (which is described in that bug) we've been using on OSX, but that didn't fix for dark theme of course.

Comment 7

[Dão Gottwald [:dao]]

(In reply to Johann Hofmann [:johannh] from comment #6)
> (In reply to Dão Gottwald [::dao] from comment #5)
> > (In reply to Mehmet from comment #4)
> > > It seems, that in FF59, the Light Theme is fixed. But the Dark Theme is
> > > still broken.
> > 
> > Would be interesting to know when exactly it got fixed so we can apply the
> > same fix to the Dark theme.
> 
> Presumably in bug 1417709 which introduced
> -moz-mac-vibrant-titlebar-light/dark to avoid the -moz-window-titlebar hack
> (which is described in that bug) we've been using on OSX, but that didn't
> fix for dark theme of course.

So enabling vibrancy in the Dark theme would fix this. We didn't do that for aesthetic reasons (bug 1386721 comment 10).

Short of enabling vibrancy, we need to fix dragging by other means. This rule is generally supposed to take care of this:

https://searchfox.org/mozilla-central/rev/588d8120aa11738657da93e09a03378bcd1ba8ec/browser/base/content/browser.css#285-287

Is this ineffective on Mac? Is window dragging also broken with third-party lightweight themes?

Comment 8

[Tim Nguyen :ntim]

> Short of enabling vibrancy, we need to fix dragging by other means.

Dragging works fine, it's just double-clicking to minimize which doesn't work.

To fix this, #TabsToolbar needs -moz-appearance: -moz-window-titlebar, or -moz-mac-vibrant-titlebar-light/dark, but either of those will change the appearance.

It would be better if the titlebar double-clicking behavior were associated with -moz-window-dragging: drag; rather than a -moz-appearance value.

Markus, what do you think >

Comment 9

[Tim Nguyen :ntim]

To be clear, atm, #TabsToolbar uses -moz-appearance: none; on the dark theme, -moz-appearance: -moz-mac-vibrant-titlebar-light; on the light theme, and -moz-mac-vibrant-titlebar-dark on the default theme.

Comment 10

[Markus Stange [:mstange]]

(In reply to Tim Nguyen :ntim from comment #8)
> It would be better if the titlebar double-clicking behavior were associated
> with -moz-window-dragging: drag; rather than a -moz-appearance value.

I think the reason that this wasn't done initially was that we had some windows with draggable bottom bars, for example the pre-arrowpanel Downloads window, and double-clicking on those wasn't supposed to do anything special. I was also under the assumption that for example the Favorites bar in Safari doesn't react to double clicks, but it appears I was wrong about that.

So it looks like there's no compelling reason for treating double click behavior separately from dragging behavior any more.

Comment 11

[Markus Stange [:mstange]]

Attachment: Bug 1401195 - Make double-clicking draggable parts of the window always do the titlebar-double-click action, regardless of whether the click targeted the titlebar or some other draggable element.

In the past there were cases where treating double click behavior differently from draggability
was necessary, but there don't seem to exist such cases any more. For example, we don't have any
windows with bottom bars / draggable status bars any more.
This change simplifies the code and frees the frontend from having to use special -moz-appearance
values to indicate titlebary parts of the window.

Review commit: https://reviewboard.mozilla.org/r/226556/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/226556/

Comment 12

[Stephen A Pohl [:spohl]]

Comment on attachment 8957649 [details]
Bug 1401195 - Make double-clicking draggable parts of the window always do the titlebar-double-click action, regardless of whether the click targeted the titlebar or some other draggable element.

https://reviewboard.mozilla.org/r/226556/#review232446

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e9db9c66ccd
Make double-clicking draggable parts of the window always do the titlebar-double-click action, regardless of whether the click targeted the titlebar or some other draggable element. r=spohl

Comment 14

[Tim Nguyen :ntim]

Thanks for looking at this so quickly!

Does this mean that the code associating different -moz-appearance values to the double click behaviour can be cleaned up ?

Comment 15

[Markus Stange [:mstange]]

If you think it's valuable, sure. Changing the list of recognized -moz-appearance values is always a bit of a hassle because you need to coordinate checkins to stylo and autoland, though...

Comment 16

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/5e9db9c66ccd

Comment 17

[Mehmet]

Great, thank you very much! Works fine in latest Nightly 60.0a1 (2018-03-10) (64-Bit).

Comment 18

[Alexandra Martin]

This issue is verified as fixed since Firefox version 60, flags will be updated.