rearranging tabs in panorama won't match tab ordering on tab bar

VERIFIED FIXED in Firefox 4.0b11

Status

defect
P2
normal
VERIFIED FIXED
9 years ago
3 years ago

People

(Reporter: blindskull13, Assigned: ttaubert)

Tracking

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

Firefox Tracking Flags

(blocking2.0 -)

Details

Attachments

(1 attachment, 5 obsolete attachments)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:2.0b8pre) Gecko/20101203 Firefox/4.0b8pre
Build Identifier: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:2.0b8pre) Gecko/20101203 Firefox/4.0b8pre

when trying to rearrange your tabs in panorama view, the tabs on the tab bar are not effected Ex. they don't rearrange to match panorama 

Reproducible: Always

Steps to Reproduce:
1. have at least three tabs open 
2. open panorama
3. move some of the tabs in panorama around in there group
4. go back to the browser view
Actual Results:  
the tabs in the tab bar do not reflect the ordering in panorama

Expected Results:  
both the tabs on the tab bar, and the tabs in panorama should have reflected the "last changed" ordering

i have reproduced this on two different machines running the same OS
(Reporter)

Updated

9 years ago
Blocks: 598154
Version: unspecified → Trunk
(Reporter)

Updated

9 years ago
Depends on: 616732
(Reporter)

Updated

9 years ago
Blocks: 616732
No longer blocks: 598154
No longer depends on: 616732
Severity: major → normal
OS: Windows 7 → All
Hardware: x86_64 → All
(Reporter)

Comment 1

9 years ago
yes, i realize you cant efficiently arrange tabs at the moment, but when you do move them in panorama, they should be able to reflect the move on the tab bar, at this current time, they don't, even when a tab only moves to the first position

Comment 2

9 years ago
Whacky, this is an unfortunate regression that I am seeing on win7 as well. When bug 587503 lands, we'll be able to debug this much more easily.

Nominating for blocking; this isn't data loss, but it's data loss-ey.
Blocks: 598154
Status: UNCONFIRMED → NEW
blocking2.0: --- → ?
Ever confirmed: true
Priority: -- → P2
Duplicate of this bug: 618471
This is in an interesting class of bugs that i consider "state loss". There's no real data loss, but there is loss of effort expended by the user at some point in time.

Because there's no real data loss, and the workaround is not unreasonable, blocking-.
blocking2.0: ? → -
Depends on: 587503

Comment 5

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

Comment 6

8 years ago
bugspam (removing b9)
No longer blocks: 598154
Duplicate of this bug: 624998
(Assignee)

Updated

8 years ago
Assignee: nobody → tim.taubert
Status: NEW → ASSIGNED
(Assignee)

Updated

8 years ago
Duplicate of this bug: 626128
Posted patch patch v1 (obsolete) — Splinter Review
Pushed to try.
Attachment #504183 - Flags: review?(ian)
(Assignee)

Updated

8 years ago
Blocks: 624998
Posted patch patch v2 (refactored) (obsolete) — Splinter Review
Refactored and added some small corrections for test for bug 586553 that covers the same area.
Attachment #504183 - Attachment is obsolete: true
Attachment #504209 - Flags: review?(ian)
Attachment #504183 - Flags: review?(ian)
Attachment #504209 - Attachment is obsolete: true
Attachment #504310 - Flags: review?(ian)
Attachment #504209 - Flags: review?(ian)
Patch v2 passed try.

Comment 13

8 years ago
Hi,
I have exactly the same problem since tabcandy was first previewed (6+ months ago).
If you recreate closed tab (CTRL+SHIFT+T), the new panorama window is created, this is annoying, but not that bad... If you move this tab to main panorama window, all tabs are rearanged, which is very serious (and of course tabs in browser no more reflect panorama ordering - until restart of firefox). I'm used to have plenty of tabs open and I know exactly where they are and this is just crazy.
Is there any way how to disable panorama at all? Is there any way I can merge easily the patch above into firefox?
4.0b9

T.
(In reply to comment #13)
> If you recreate closed tab (CTRL+SHIFT+T), the new panorama window is created,
> this is annoying, but not that bad...

This is bug 624265 which has been fixed in the nightly, and will be in beta 10.
Comment on attachment 504310 [details] [diff] [review]
patch v2b (small fixes in head.js)

Very nice. Comments: 

>+      let start = index ? indices[index-1] : -1;
>+      let end = index+1 < indices.length ? indices[index+1] : Infinity;
>+      let targetRange = new Range(start, end);
>+
>+      if (!targetRange.contains(tabItem.tab._tPos)) {
>+        gBrowser.moveTabTo(tabItem.tab, start+1);

The only reason we can get away with start being indices[index - 1] is because we know that tab's already taken that index, so we won't have it. It seems like it would be a little clearer if start was indices[index - 1] + 1, like so: 

      let start = index ? (indices[index - 1] + 1) : 0;
      let end = index + 1 < indices.length ? (indices[index + 1] - 1) : Infinity;
      let targetRange = new Range(start, end);

      if (!targetRange.contains(tabItem.tab._tPos)) {
        gBrowser.moveTabTo(tabItem.tab, start);
        
It works either way (since there are no duplicated indices), but I feel this is a little cleaner. What do you think?

>+    tests.push([]);

It took me a moment to realize this was your clever way to delete that group. Probably deserves a comment to that effect.

R+ with those addressed
Attachment #504310 - Flags: review?(ian) → review+
Posted patch patch v3 (obsolete) — Splinter Review
(In reply to comment #15)
> It works either way (since there are no duplicated indices), but I feel this is
> a little cleaner. What do you think?

I had in mind that there are no duplicate indices while writing but anyone reading this could not - so this is really a bit cleaner, good catch.

> It took me a moment to realize this was your clever way to delete that group.
> Probably deserves a comment to that effect.

Well, I wanted to test how empty groups behave, too. But yeah a neat side-effect was that the group disappeared :) I added comment to clarify this.
Attachment #504310 - Attachment is obsolete: true
Attachment #504900 - Flags: approval2.0?
Thanks for pushing this forward Tim. Are the changes in the latest patch substantial enough to require another tryserver run?

Comment 18

8 years ago
minefield 4.0b10pre (2011-01-19)
still not working correctly:(
(In reply to comment #17)
> Thanks for pushing this forward Tim. Are the changes in the latest patch
> substantial enough to require another tryserver run?

I pushed to try yesterday. There was a leak on Win (debug). I pushed again (win only) to see if this is temporary.
(In reply to comment #18)
> minefield 4.0b10pre (2011-01-19)
> still not working correctly:(

Tomas, that is expected this this bug is resolved.  So please wait till we know when to check the nightly.
(In reply to comment #20)
> (In reply to comment #18)
> > minefield 4.0b10pre (2011-01-19)
> > still not working correctly:(
> 
> Tomas, that is expected this this bug is resolved. 

That should say, until this bug is resolved.  Sorry for the spam.
This seems to permanently leak on win32 :(
Posted patch patch v4 (obsolete) — Splinter Review
Added some small optimizations to the algorithm. Hopefully this does not leak anymore. Pushed to try again.
Attachment #504900 - Attachment is obsolete: true
Attachment #505323 - Flags: approval2.0?
Attachment #504900 - Flags: approval2.0?
Duplicate of this bug: 627366
(In reply to comment #4)
> Because there's no real data loss, and the workaround is not unreasonable,
> blocking-.

I disagree.  IMO it would be a mistake to release 4 without fixing
this.  It blemishes what is otherwise a killer new feature.

I find it confusing.  The worst thing is that, because the order of
thumbnails and tabs is currently not reliably connected, it made me
think for a while that my mental model of how the user interface works
(in this respect) is missing some crucial detail.  It makes it appear
that there's some magic other piece of user-interface state that
controls the mapping, and if I just tried harder I could find out what
I need to do to access it.  Whereas, in reality, there isn't.

I believe this will confuse users en masse.

It also makes Panorama less useful.  I want to rearrange my tabs for a
reason -- to make finding them easier -- and at the moment I simply
can't do that reliably.
Approval, please.
Blocks: 627096
No longer blocks: 608028
(In reply to comment #23)
> Created attachment 505323 [details] [diff] [review]
> patch v4

Works for me! (x64-linux m-c build).

Comment 28

8 years ago
(In reply to comment #25)
> (In reply to comment #4)
> > Because there's no real data loss, and the workaround is not unreasonable,
> > blocking-.
> 
> I disagree.  IMO it would be a mistake to release 4 without fixing
> this.  It blemishes what is otherwise a killer new feature.
> 
> I find it confusing.  The worst thing is that, because the order of
> thumbnails and tabs is currently not reliably connected, it made me
> think for a while that my mental model of how the user interface works
> (in this respect) is missing some crucial detail.  It makes it appear
> that there's some magic other piece of user-interface state that
> controls the mapping, and if I just tried harder I could find out what
> I need to do to access it.  Whereas, in reality, there isn't.
> 
> I believe this will confuse users en masse.
> 
> It also makes Panorama less useful.  I want to rearrange my tabs for a
> reason -- to make finding them easier -- and at the moment I simply
> can't do that reliably.

Can't agree more. This all panorama thing is now just useless, every time I move the tab, it's all mixed up. The whole idea was to make things more simple, visual and efficient, unfortunately now it's just the opposite:( Hopefully there will be an option to disable it completely, otherwise I'll have to stick with F3 until this is resolved.

To Julian Seward: 21-Jan-2011 06:32 (win64) - still not working.
(In reply to comment #26)
> Approval, please.

The trick is to poke specific people about approval and/or make sure they are CC'ed. I'm guessing the approval queue is not being triaged as actively as you might think.

CCing dolske to come try and beat dietrich and beltzner to setting the approval flag.
CCing dolske for real this time...
Attachment #505323 - Flags: approval2.0? → approval2.0+
Passed try (only hit bug 626956).
Attachment #505323 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
Keywords: checkin-needed

Comment 33

8 years ago
http://hg.mozilla.org/mozilla-central/rev/b38ba02fdeb4
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b10
Target Milestone: Firefox 4.0b10 → ---
Target Milestone: --- → Firefox 4.0b11
verified with nightly build of minefield.
Status: RESOLVED → VERIFIED
(Assignee)

Updated

8 years ago
Depends on: 635362
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.