Dragging tab between groups doesn't work

VERIFIED FIXED in Firefox 4.0b11

Status

defect
P2
normal
VERIFIED FIXED
9 years ago
3 years ago

People

(Reporter: dietrich, Assigned: ttaubert)

Tracking

Trunk
Firefox 4.0b11
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ux][polish])

Attachments

(1 attachment, 5 obsolete attachments)

(Reporter)

Description

9 years ago
sometimes i'll drag a tab from group A to group B.

expected: the tabs in group A rearrange to compensate for the now-missing tab. the tabs in group B rearrange to compensate for the newly-arrived tab.

actual: nothing moves in group A, and there's a hole where the tab used to be. nothing moves in group B, and the tab stays wherever i dropped it.

error in the console when this happens:

GroupItem.add error; tabview assert: shouldn't already be in another groupItem([object MouseEvent])@chrome://browser/content/tabview.js:3351
([object MouseEvent])@chrome://browser/content/tabview.js:1640
([object MouseEvent])@chrome://browser/content/tabview.js:675
@:0
Blocks: 598154
Priority: -- → P2
If this is the same bug I'm experiencing, it appears to only happen when Panorama is busy loading thumbnail previews. Once all the previews are loaded, there doesn't appear to be a problem.

Though I changed the bug summary, I hesitate to add the part about previews until it is confirmed that these are indeed the same bug.

Dietrich, can you confirm that it is only a problem while tab previews are loading?
OS: Linux → All
Hardware: x86 → All
Summary: sometimes dragging between groups results in a broken state → Dragging tab between groups doesn't work

Comment 2

8 years ago
bugspam (moving b9 to b10)
Blocks: 608028

Comment 3

8 years ago
bugspam (removing b9)
No longer blocks: 598154
I experience this bug quite often. This happens even when previews are not loading - on average in 1 of 10 tries. The trick is to quickly drag a tab and drop it so that almost no mouseevents are fired. So this might as well happen even more when firefox or the whole computer is under load.
Assignee: nobody → tim.taubert
Status: NEW → ASSIGNED
Posted patch patch v1 (obsolete) — Splinter Review
Pushed to try.
Attachment #504196 - Flags: review?(ian)
Comment on attachment 504196 [details] [diff] [review]
patch v1

>+function showTabView(callback) {
>+function hideTabView(callback) {

Please add an optional win argument, probably after callback, so that we can specify different windows as well. See some of the other head.js functions for examples.
(In reply to comment #6)
> Please add an optional win argument, probably after callback, so that we can
> specify different windows as well. See some of the other head.js functions for
> examples.

Good catch! Added.
Attachment #504196 - Attachment is obsolete: true
Attachment #504210 - Flags: review?(ian)
Attachment #504196 - Flags: review?(ian)
Comment on attachment 504210 [details] [diff] [review]
patch v1b (small fixes in head.js)

>+  if (TabView.isVisible()) {

>+  TabView.show();

>+  if (!TabView.isVisible()) {

>+  TabView.hide();

These need to be win.TabView, else they'll just be accessing the main window's TabView.
Attachment #504210 - Attachment is obsolete: true
Attachment #504254 - Flags: review?(ian)
Attachment #504210 - Flags: review?(ian)
Attachment #504254 - Attachment is obsolete: true
Attachment #504307 - Flags: review?(ian)
Attachment #504254 - Flags: review?(ian)
Passed try.
Comment on attachment 504307 [details] [diff] [review]
patch v1d (small fixes in head.js)

Looking good. 

>+    is(targetGroup.getChildren().length, 2, 'target group has two tabs');

Go ahead and also check that sourceGroup has been closed.

R+ with that
Attachment #504307 - Flags: review?(ian) → review+
Posted patch patch v2 (obsolete) — Splinter Review
Attachment #504307 - Attachment is obsolete: true
Attachment #505251 - Flags: approval2.0?
Approval please.
Whiteboard: [ux][polish]
Attachment #505251 - Flags: approval2.0? → approval2.0+
Attachment #505251 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
Keywords: checkin-needed

Comment 17

8 years ago
bugspam. Moving b10 to b11
Blocks: 627096

Comment 18

8 years ago
bugspam. Removing b10
No longer blocks: 608028

Comment 19

8 years ago
http://hg.mozilla.org/mozilla-central/rev/979d2ebe8b7e
Flags: in-testsuite+
Keywords: checkin-needed
Target Milestone: --- → Firefox 4.0b10

Updated

8 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(In reply to comment #19)
> http://hg.mozilla.org/mozilla-central/rev/979d2ebe8b7e

Don't think this will make it into beta 10.
Target Milestone: Firefox 4.0b10 → ---
Target Milestone: --- → Firefox 4.0b11
verified with nightly build of minefield.
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.