Closed Bug 635397 Opened 11 years ago Closed 11 years ago
when tabbar and menubar have unified appearance, tabbar should be just as draggable as menubar
957 bytes, patch
|Details | Diff | Splinter Review|
1.44 KB, patch
|Details | Diff | Splinter Review|
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.
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.
Component: General → Theme
QA Contact: general → theme
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)
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...
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.
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.
So what should happen here? I think this is a pretty bad regression from bug 580970.
(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.)
(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.)
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.
(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 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-
Good point. That works great.
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+
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0
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
(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 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)
(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.
You need to log in before you can comment on or make changes to this bug.