465332 - drag to tear off tabs should allow dropping a tab on the bookmark toolbar

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect, P2)

Description

[Damian Shaw [Quan]]

Steps to reproduce

1. Drag a tab in to the bookmark toolbar
2. Tab opens in a new window

Expected Result:

Bookmark is created, same as bug 465256?

Comment 1

[Asaf Romano (gone)]

Fixed.

Comment 2

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081118 Minefield/3.1b2pre ID:20081118125341

Still seeing this

Comment 3

[Asaf Romano (gone)]

Er, yeah, I read this bug wrong. That's by design given the new behavior.

Comment 4

[Peter van der Woude [:Peter6]]

Damian, you can always drag from the locationbar to the bookmarks toolbar to create a bookmark there.

Comment 5

[Damian Shaw [Quan]]

I know what you  'can do' to make a bookmark. But it doesn't seem intuitive to me that dragging a tab on to the bookmark toolbar or the bookmark menu doesn't create a new bookmark. I understand dragging to 'open areas' makes a new window but these are well defined areas with well defined purposes.

Would you please reconsider this.

Comment 6

[Simon Bünzli]

REOPENING as this is a UI regression in two ways:

* A tab is a significantly bigger target than the favicon, so simpler to drag.

* Background tabs can no longer be bookmarked in one swoop without always first selecting the tab and then aiming for the favicon (resp. added to ScrapBook or whatever cool things extensions could do with tabs).

Mike, Alex: Are we really intentionally accepting these two regressions?

Comment 8

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

(In reply to comment #6)
> Mike, Alex: Are we really intentionally accepting these two regressions?

I don't think we should have to, no.

(In reply to comment #3)
> Er, yeah, I read this bug wrong. That's by design given the new behavior.

Well, it's by *effect* from the new behaviour, but I think there's more sensible things we can do here. It's fine for beta 2, but we should:

 - create a bookmark when there is a valid drop target on the bookmark toolbar (ie: when the cursor would have changed to an insertion marker)

 - probably consider adjusting the amount of vertical drag required to result in tabs being torn off

(Morphing this bug a little bit to represent that ...)

Comment 9

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

In terms of vertical sensitivity, I think we should probably use half the tab height (so you have to drag the tab at least half way down past the bottom/left/right edge of the tab strip)

Also, ESC should cancel the action if it's pressed before the tab is dropped.

Comment 10

[Damian Shaw [Quan]]

Given the massive morph of this bug should it also include bug 465910?

Comment 11

[Simon Bünzli]

(In reply to comment #8)
>  - create a bookmark when there is a valid drop target on the bookmark toolbar

And in the bookmarks sidebar and bookmarks menu as well, I presume.

(In reply to comment #9)
> Also, ESC should cancel the action if it's pressed before the tab is dropped.

That's being tracked in bug 465608.

Comment 12

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

(In reply to comment #9)
> In terms of vertical sensitivity, I think we should probably use half the tab
> height (so you have to drag the tab at least half way down past the
> bottom/left/right edge of the tab strip)
> 
> Also, ESC should cancel the action if it's pressed before the tab is dropped.

Ooops - this is bug 465346, unmorphing ;)

Comment 13

[eleven81]

I have already isolated the regression window the best that I could, and made note of this in bug 465972.  I'll duplicate that information here.  I found that the bug is not present in build 2008-11-14-03-mozilla-central, but the bug is present in build 2008-11-15-15-mozilla-central.

In build 2008-11-14-03-mozilla-central and previous builds, a user could already create a bookmark by dragging a tab and dropping it onto the bookmarks toolbar.

Comment 14

[Dirkjan Ochtman (:djc)]

So is this bug about increasing the vertical drop target for dragging tabs within the window? That is, I'm not accidentally dragging a tab, but I do want to keep it in my current window. If I try to do so, the vertical area that I apparently need to target should be a little larger than just the tab bar, IMO. Fitt's Law, and all that.

Bug 340463 and bug 450915 also seem related.

Comment 15

[Michael Lefevre]

(In reply to comment #14)
> So is this bug about increasing the vertical drop target for dragging tabs
> within the window?

No, that sounds like bug 465346.  See also the various dependencies of bug 225680 https://bugzilla.mozilla.org/showdependencytree.cgi?id=225680.  The two bugs you referenced are (or hopefully will be) obsolete after the new changes.

Comment 16

[Dirkjan Ochtman (:djc)]

Michael, it doesn't really sound like bug 465346 to me, since that seems to be about a general treshold keeping it from detaching. I'm talking about the drop target for drag & drop within the window, where I want to move the tab along the tab bar. If I let the mouse cursor drop a little into the content window (just a few pixels south of the tab bar), this is seen as a detach event, when I really still want to move it within the window to another place on the tab bar.

Comment 17

[Michael Lefevre]

(In reply to comment #16)
> Michael, it doesn't really sound like bug 465346 to me, since that seems to be
> about a general treshold keeping it from detaching. I'm talking about the drop
> target for drag & drop within the window

Both issues are discussed in the comments of that bug. I would hope that the work to fix bug 465184 and bug 465346 will solve the problem you are talking about. If not, you could file another bug - I'm not sure filing yet another similar bug before the work on the others has happened will be useful.

Comment 18

[eleven81]

Bug 465332 is a duplicate of this bug.  The expected results of this bug and bug 465332 are in the same vein: "dragging a tab to the bookmarks toolbar should create a bookmark for the tab."

However, the last several comments discuss dragging a tab to shuffle the order of the tabs on the tab bar.  There is already a bug for that behavior: bug 450915.

Comment 19

[Dirkjan Ochtman (:djc)]

eleven81: bug 465332 *is* this bug. Did you intend to mention some other bug?

Comment 20

[eleven81]

My mistake.  I meant bug 465972.  Thanks Dirkjan!

Comment 21

[Nochum Sossonko [:Natch]]

Attachment: re-adding the flavors.

mano, this seems to fix a good number of the regressions, while still detaching the tab when dragged over the content window and anywhere else that doesn't support uri dropping. The only caveat is that dragging a tab to an input puts the uri in the input field rather than detaching it.

Comment 22

[IU]

(In reply to comment #21)
> The only caveat is that dragging a tab to an input puts
> the uri in the input field rather than detaching it.

Please ALLOW dragging a tab to a textarea place the URI in the field.  I don't understand why you would want to disallow this.

Comment 23

[Damian Shaw [Quan]]

(In reply to comment #21)
> The only caveat is that dragging a tab to an input puts
> the uri in the input field rather than detaching it.

Isn't that a good thing? See Bug 467376

Comment 24

[Nochum Sossonko [:Natch]]

(In reply to comment #23 and comment #22)
s/caveat/difference

I don't know what the decision is for the input field case, could be good or bad, what I really meant is that the difference is this. This patch fixes bug 465182, bug 465332, bug 465910, bug 467376 and bug 467461.

Comment 25

[alex_mayorga]

(In reply to comment #22)
> (In reply to comment #21)
> > The only caveat is that dragging a tab to an input puts
> > the uri in the input field rather than detaching it.
> 
> Please ALLOW dragging a tab to a textarea place the URI in the field.  I don't
> understand why you would want to disallow this.

+1 for "ALLOW dragging a tab to a textarea place the URI in the field"

Comment 26

[Damian Shaw [Quan]]

bug 468553 is also worth looking at, it's kind of one of the cases I orriginally meant with this bug before it got morphed.

Comment 27

[eyal gruss]

(In reply to comment #4)
> Damian, you can always drag from the locationbar to the bookmarks toolbar to
> create a bookmark there.

do you mean drag the favicon from the location bar?

Comment 28

[Peter van der Woude [:Peter6]]

(In reply to comment #27)
> (In reply to comment #4)
> > Damian, you can always drag from the locationbar to the bookmarks toolbar to
> > create a bookmark there.
> 
> do you mean drag the favicon from the location bar?
Tes

Comment 31

[Guillermo Marraco]

(In reply to comment #4)
> Damian, you can always drag from the location bar to the bookmarks toolbar to
> create a bookmark there.

No good option. You can't drag a tab other than active tab.

Also, to open the tab on a new window, you can right click with mouse, and select "open on a new window".

The Firefox 3.1 behavior is totally unintuitive.

Also, is inconvenient OOP coding to drop a tab on a toolbar, and make her take a work unrelated with a tool bar. It can seed hard to track bugs.

Comment 33

[Nochum Sossonko [:Natch]]

I don't think this is an optimal patch, as IIRC (haven't tested this patch recently), this creates a shortcut when dragging to windows explorer (ie the desktop), or for that matter any application that accepts dropping a uri will block the tear. onemen.one had a better patch in bug 465184 but mano said he didn't like it and that he had a better patch, so we should probably wait for him?

Comment 34

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 351096 [details] [diff] [review]
re-adding the flavors.

Canceling review per comment 33.

Comment 35

[Nochum Sossonko [:Natch]]

I'd venture to take a stab at some of the tab dragging bugs, but I don't want to conflict with mano's patch. Any chance we'll get to see that soon? How many bugs does it resolve? IOW, can we get a status update?

Comment 37

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

Verified on trunk and 1.9.1 with:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090125
Shiretoko/3.1b3pre (.NET CLR 3.5.30729) ID:20090125052901

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre)
Gecko/20090126 Shiretoko/3.1b3pre Ubiquity/0.1.5 ID:20090126020313

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090125
Minefield/3.2a1pre ID:20090125034316

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre)
Gecko/20090126 Minefield/3.2a1pre ID:20090126020316

Comment 38

[Aakash Desai [:aakashd]]

Test case https://litmus.mozilla.org/show_test.cgi?id=7488 has been updated for regression testing on rel 3.1 test run.

Comment 40

[Charles]

What I think would make sense is:

onDragTab(){
     if(releaseMouseButton==overBookmarkPanel){
          addBookmark;
     } else {
          tearOffTab;
}

:-)