625424 - Dropping a tab onto the "undo close group" dialog adds the tab to that group

Status: VERIFIED FIXED

(Firefox Graveyard :: Panorama, defect, P2)

Description

[Kevin Hanes]

- face palm -

Note that this tab goes away when the group gets deleted.

Comment 1

[Raymond Lee [:raymondlee]]

I am not able to reproduce this on Mac OSX.  Does this happen in other platforms or I miss something?

Comment 2

[Raymond Lee [:raymondlee]]

I can reproduce now.  Just need to put the tab on top of the "undo close group" and wait until the "undo close group" gets removed automatically.

Comment 3

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

Don't think that's gonna be a common user interaction, doesn't really cause dataloss.

Comment 4

[Ian Gilman [:iangilman]]

(In reply to comment #3)
> Don't think that's gonna be a common user interaction, doesn't really cause
> dataloss.

Well, the thing is, you just need to drop the tab near the "undo close group" button... It's certainly possible to do by accident, and it does result in the tab being killed.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v1

Pushed to try.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

Passed try!

Comment 7

[Ian Gilman [:iangilman]]

Comment on attachment 504965 [details] [diff] [review]
patch v1

Beautiful

Comment 8

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

Comment on attachment 504965 [details] [diff] [review]
patch v1

a=beltzner, thanks for the tests!

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch for checkin

Comment 10

[Ian Gilman [:iangilman]]

http://hg.mozilla.org/mozilla-central/rev/8cfa54285e34

Comment 11

[Tim Taubert [:ttaubert] (inactive)]

Backed out: http://hg.mozilla.org/mozilla-central/rev/5a0f46c26d12

Causes orange on windows.

Comment 12

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v2

Restructured the test and sent to try.

Comment 13

[Ian Gilman [:iangilman]]

Comment on attachment 507218 [details] [diff] [review]
patch v2

Looks good to me, assuming try likes it.

Comment 14

[Tim Taubert [:ttaubert] (inactive)]

Passed try.

Comment 15

[Shawn Wilsher :sdwilsh]

How did this break before when it landed on mozilla-central after it passed try?  How are we confident that that won't happen again?

Comment 16

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

Comment on attachment 507218 [details] [diff] [review]
patch v2

a=beltzner

Comment 17

[Shawn Wilsher :sdwilsh]

(In reply to comment #16)
> a=beltzner
comment 15?

Comment 18

[Tim Taubert [:ttaubert] (inactive)]

(In reply to comment #15)
> How did this break before when it landed on mozilla-central after it passed
> try?  How are we confident that that won't happen again?

I believe the test failed because it simulates dragging a tabItem onto a closed group and the group size and position can be affected by previous tests. I modified the test to run in a new window with a clean tabview setup so that the position and size of the groups are not changed by previous tests.

Comment 19

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch for checkin

Comment 20

[Jonathan Watt [:jwatt]]

Pushed http://hg.mozilla.org/mozilla-central/rev/336d5906cb0f

Comment 21

[Tomas]

Is that mean fixed?

Comment 22

[Michael Yoshitaka Erlewine [:mitcho]]

Verified that this is fixed on trunk. Filed a follow up, though: bug 603789.

Comment 23

[Tracy Walker [:tracy]]

with nightly build of minefield: 

dropping a tab on an "undo close group" doesn't put it in that group. However if you wait several seconds, the undo close group disappears.  unexpected data loss.  Is that part of any follow ups?

Comment 24

[Tim Taubert [:ttaubert] (inactive)]

(In reply to comment #23)
> dropping a tab on an "undo close group" doesn't put it in that group. However
> if you wait several seconds, the undo close group disappears.  unexpected data
> loss.  Is that part of any follow ups?

This is expected behavior. Closing groups cannot be undone after waiting 15s. Feel free to open a new bug if you think that should be discussed again :)

Comment 25

[Tracy Walker [:tracy]]

verified with minefield build of 20110217