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

VERIFIED FIXED in Firefox 4.0

Status

()

Firefox
Theme
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

(Depends on: 1 bug, {regression})

Trunk
Firefox 4.0
All
Linux
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

7 years ago
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.
(Assignee)

Comment 1

7 years ago
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.
(Assignee)

Updated

7 years ago
Keywords: regression
(Assignee)

Updated

7 years ago
Component: General → Theme
QA Contact: general → theme
(Assignee)

Comment 2

7 years ago
Created attachment 513633 [details] [diff] [review]
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).
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
Attachment #513633 - Flags: review?(dao)
(Assignee)

Updated

7 years ago
status2.0: --- → ?
(Assignee)

Updated

7 years ago
status2.0: ? → ---
(Assignee)

Comment 3

7 years ago
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 on attachment 513633 [details] [diff] [review]
patch

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

Comment 5

7 years ago
Hmm.  Should the double-click to create a new tab behavior be disabled in this unified case?  (Is it on other platforms?)
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.
(Assignee)

Comment 7

7 years ago
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.
I get very inconsistent behavior with the patch. Double clicks seem to do nothing and triple clicks open a new tab.
(Assignee)

Comment 9

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

Comment 10

7 years ago
(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.
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.)
(Assignee)

Comment 12

7 years ago
(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.)
(Assignee)

Comment 13

7 years ago
Created attachment 513886 [details] [diff] [review]
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.
Attachment #513886 - Flags: review?(dao)
(Assignee)

Comment 14

7 years ago
(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.
(Assignee)

Comment 15

7 years ago
Created attachment 514060 [details] [diff] [review]
disable doubleclick behavior exactly when tabbar makes window draggable
Attachment #513886 - Attachment is obsolete: true
Attachment #514060 - Flags: review?(dao)
Attachment #513886 - Flags: review?(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)|.
Attachment #514060 - Flags: review?(dao) → review-
(Assignee)

Comment 17

7 years ago
Created attachment 514068 [details] [diff] [review]
disable doubleclick behavior exactly when tabbar makes window draggable

Good point.  That works great.
Attachment #514060 - Attachment is obsolete: true
Attachment #514068 - Flags: review?(dao)

Updated

7 years ago
Attachment #514068 - Flags: review?(dao) → review+

Updated

7 years ago
Attachment #513633 - Flags: review?(dao) → review+
(Assignee)

Updated

7 years ago
Attachment #513633 - Flags: approval2.0?
(Assignee)

Updated

7 years ago
Attachment #514068 - Flags: approval2.0?
(Assignee)

Comment 18

7 years ago
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 on attachment 513633 [details] [diff] [review]
patch

a=beltzner
Attachment #513633 - Flags: approval2.0? → approval2.0+
Attachment #514068 - Flags: approval2.0? → approval2.0+
(Assignee)

Comment 20

7 years ago
http://hg.mozilla.org/mozilla-central/rev/c011e21f29f8
http://hg.mozilla.org/mozilla-central/rev/a820f7f5e50e
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0

Comment 21

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

Verified issue and it's no longer present.
Status: RESOLVED → VERIFIED

Comment 22

7 years ago
(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?

Updated

7 years ago
Blocks: 637328
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.
Attachment #514068 - Flags: ui-review?(limi)

Comment 24

7 years ago
(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.
(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.
Depends on: 641490

Updated

7 years ago
Depends on: 674847

Updated

7 years ago
No longer depends on: 674847
You need to log in before you can comment on or make changes to this bug.