635397 - when tabbar and menubar have unified appearance, tabbar should be just as draggable as menubar

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Since bug 580970 landed yesterday, the tabbar and the menubar have a unified appearance.  However, underneath, the menubar is draggable (thanks to bug 566480) but the tabbar is not, which means that there's an invisible line between what you can drag and what you can't drag.

I think this is quite bad.

Steps to reproduce:
 1. use a theme with unified titlebar-menubar appearance, such as Ambiance or Radiance on Ubuntu.
 2. open a Firefox window with one tab
 3. try dragging the Firefox window by dragging on the empty space next to the tab

Actual results: nothing

Expected results:  window moves, just like it does when dragging on the empty space next to the menu items.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I suspect this can be fixed with a style rule similar to the rule added to global.css in bug 566480.  I'll try to remember to try that once I update my tree.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

This fixes it.

I think this is pretty safe, since we apply the same binding to the tabbar on Windows when (-moz-windows-compositor) is true:
http://hg.mozilla.org/mozilla-central/file/a63e2fb78899/browser/themes/winstripe/browser/browser-aero.css#l200
(and I suspect we also do on Mac, via a much more general rule).

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Note that the reason this is conditioned on :-moz-system-metric(menubar-drag) is that that's the condition under which the menubar is draggable -- and it's generally true for those themes in which titlebar and menubar have a unified appearance.

Comment 4

[Dão Gottwald [:dao]]

Comment on attachment 513633 [details] [diff] [review]
patch

This seems to handle double clicks on the tab strip quite bad...

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Hmm.  Should the double-click to create a new tab behavior be disabled in this unified case?  (Is it on other platforms?)

Comment 6

[Dão Gottwald [:dao]]

Double-click maximizes the window on Windows when using the toolbar-drag binding. I'm not sure if this is the expected behavior on Linux, but if it isn't, double-click should continue to open a tab.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The titlebar doubleclick behavior on my setup (Ubuntu + Ambiance + metacity) is also to maximize, but I'm not sure how universal that is.

That said, currently it does continue to create a new tab for me, unless I initiate a drag during the double-click.

Comment 8

[Dão Gottwald [:dao]]

I get very inconsistent behavior with the patch. Double clicks seem to do nothing and triple clicks open a new tab.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So what should happen here?  I think this is a pretty bad regression from bug 580970.

Comment 10

[Chris Coulson]

(In reply to comment #7)
> The titlebar doubleclick behavior on my setup (Ubuntu + Ambiance + metacity) is
> also to maximize, but I'm not sure how universal that is.
> 

This is the default behaviour for Compiz and Metacity and Mutter (ie, classic GNOME 2, gnome-shell and Unity). I'm guessing it's probably the same default for other window managers too.

Comment 11

[Dão Gottwald [:dao]]

We need a patch that makes double click work one way or another. I don't have an opinion about which way. (My earlier question was what's expected to happen when double clicking on a unified menu bar, not on the titlebar itself.)

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #11)
> We need a patch that makes double click work one way or another. I don't have
> an opinion about which way. (My earlier question was what's expected to happen
> when double clicking on a unified menu bar, not on the titlebar itself.)

In other GNOME apps, double-clicking on the unified menubar does not have the same double-click behavior as the titlebar.  The unification only applies to the appearance and the drag behavior.  (Personally, I'd consider this a GNOME bug, though.)

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: start drag on dragstart rather than mousedown

This fixes the double-clicking behavior for me, I think.  (The behavior I was seeing was that double-click to open a new tab worked more than half of the time, but not all the time.  This fixes that.)

This just defers handing off the drag to the OS until we've decided that there's a drag gesture involved, which means we won't do so for a double-click.

There is a tradeoff here -- it means that we don't show the drag mouse-cursor on mousedown, but only show it after we've decided there's a drag involved.

The event struct type checks are safe since nsDragEvent inherits from nsMouseEvent.

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #13)
> There is a tradeoff here -- it means that we don't show the drag mouse-cursor
> on mousedown, but only show it after we've decided there's a drag involved.

Which, fwiw, is pretty annoying.

Personally, I'd rather go the other way and disable double-click to open new tab.  (I didn't even know that feature existed.)  But I don't know if that will produce screams from people who depended on it.

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: disable doubleclick behavior exactly when tabbar makes window draggable

Comment 16

[Dão Gottwald [:dao]]

Comment on attachment 514060 [details] [diff] [review]
disable doubleclick behavior exactly when tabbar makes window draggable

>+        // Disable this on GTK2 when the menubar is draggable, since (a)
>+        // the menubar and tabbbar have unified appearance and should
>+        // thus not have different behavior (though this condition alone
>+        // applies to more cases) and (b) it interacts badly with the
>+        // drag handling that we use for dragging either one.
>+        if (this.tabbrowser
>+                .mozMatchesSelector(":-moz-system-metric(menubar-drag)"))
>+          return;

This isn't quite right, as we don't want this for tabs on bottom, and a third-party Firefox theme might handle things entirely different. I think what you really want here is |if (this.parentNode._dragBindingAlive)|.

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: disable doubleclick behavior exactly when tabbar makes window draggable

Good point.  That works great.

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This patches are relatively straightforward patches fixing a regression from the theme changes late last week (described in comment 0).  The first patch (see comment 2) applies a binding to the tabbar that we already apply to the tabbar on other platforms.  The second patch disables the double-click for new-tab behavior in exactly the cases where that binding is applied.

Comment 19

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 513633 [details] [diff] [review]
patch

a=beltzner

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://hg.mozilla.org/mozilla-central/rev/c011e21f29f8
http://hg.mozilla.org/mozilla-central/rev/a820f7f5e50e

Comment 21

[George Carstoiu]

Mozilla/5.0 (X11; Linux i686; rv:2.0b12) Gecko/20100101 Firefox/4.0b12

Verified issue and it's no longer present.

Comment 22

[Daniel C]

(In reply to comment #14)
> Personally, I'd rather go the other way and disable double-click to open new
> tab.  (I didn't even know that feature existed.)  But I don't know if that will
> produce screams from people who depended on it.

Almost certainly. It is a useful feature for those that have used it for many years, and it has been disabled just to fix a cursor display issue, which I think is a mistake.

Is it really a good idea to remove a feature like this so late on in the cycle without any visible research?

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 514068 [details] [diff] [review]
disable doubleclick behavior exactly when tabbar makes window draggable

Why this patch never got an ui-review? It changes a key feature for users which we have supported since the beginning of Firefox. Even with the patch already landed, I would like to get feedback from UX.

Comment 24

[Yann Brelière]

(In reply to comment #23)
I agree that, even if the tab bar is unified and draggable, it shouldn't prevent from double-clicking to open a new tab.

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #23)
> Why this patch never got an ui-review? It changes a key feature for users which
> we have supported since the beginning of Firefox. Even with the patch already
> landed, I would like to get feedback from UX.

Also keep in mind that we have the exact same behavior for drag and double click on OS X, where all toolbars are also unified.