656222 - moveTabBackward/moveTabForward should skip hidden tabs

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[ithinc]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:6.0a1) Gecko/20110510 Firefox/6.0a1
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:6.0a1) Gecko/20110510 Firefox/6.0a1

When there are hidden tabs next to the current tab, moveTabBackward/moveTabForward has no effect until another visible tab is encountered.


Reproducible: Always

Steps to Reproduce:
1. Open some tabs and focus the first one
2. Move the right tab of the current tab to a new group
3. Focus the current tab itself (Ctrl+K to search bar, then Tab)
4. Ctrl+Right Arrow, no effect
5. Ctrl+Right Arrow again, it works



Expected Results:  
The hidden tabs should be skipped.

Comment 1

[George Carstoiu]

Mozilla/5.0 (Windows NT 5.1; rv:6.0a1) Gecko/20110503 Firefox/6.0a1

I am unable to reproduce the issue you have logged. 

Can you please verify it on a clean profile:
http://support.mozilla.com/en-US/kb/Managing%20profiles

Comment 2

[ithinc]

(In reply to comment #1)
> I am unable to reproduce the issue you have logged. 

Maybe you didn't follow the STR exactly.

Comment 3

[ithinc]

Attachment: Skip hidden tabs when moving tab backward/forward

Comment 4

[Vlad [QA]]

I've tried to reproduce this but i couldn't.
When in panorama, wit ctrl+right/left arrow i can scroll trough all tabs.
The same behavior on Win 7 as well.

Buil ID is: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0a1) Gecko/20110622 Firefox/7.0a1

Comment 5

[ithinc]

(In reply to comment #4)
> I've tried to reproduce this but i couldn't.
> When in panorama, wit ctrl+right/left arrow i can scroll trough all tabs.
> The same behavior on Win 7 as well.
> 
> Buil ID is: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0a1)
> Gecko/20110622 Firefox/7.0a1

You need to focus the tab itself first. See step 3 in comment 0 (tabs on bottom), or press F6 (tabs on top).

Comment 6

[YUKI "Piro" Hiroshi]

This still happen on the latest Nightly.
Mozilla/5.0 (Windows NT 6.0; rv:11.0a1) Gecko/20111202 Firefox/11.0a1

Detailed steps to reproduce:

1. Start Nightly with clean profile. You'll see tabs on top,
   and there is just one tab (about:home).
2. Press Ctrl-T to open a new tab (it is the second tab).
     [about:home][about:blank(selected)]
3. Press Ctrl-T to open a new tab again (it is the third tab).
   Then, you have three tabs.
     [about:home][about:blank(1)][about:blank(2)(selected)]
4. Right-click on the second tab, and choose "Move to Group" => "New Group"
   from the context menu.
   Then, the second tab becomes hidden:
     all tabs:
       [about:home][about:blank(1)(hidden)][about:blank(2)(selected)]
     visible tabs:
       [about:home][about:blank(2)(selected)]
5. Press Ctrl-1 to switch to the first tab.
     all tabs:
       [about:home(selected)][about:blank(1)(hidden)][about:blank(2)]
     visible tabs:
       [about:home(selected)][about:blank(2)]
6. Press F6 key. Then, a focus ring appears on the first tab.
   Now the tab has focus.
     all tabs:
       [about:home(selected,focused)][about:blank(1)(hidden)][about:blank(2)]
     visible tabs:
       [about:home(selected,focused)][about:blank(2)]
7. Press Ctrl-Right(right arrow key). Then "gBrowser.moveTabForward()" is called.

Actual result:
  Nothing happens visually. The focused tab was actually moved, but
  moveTabForward() simply moved the tab to the right of the next hidden tab.
  Visual tab order doesn't change.
    all tabs:
      [about:blank(1)(hidden)][about:home(selected,focused)][about:blank(2)]
    visible tabs:
      [about:home(selected,focused)][about:blank(2)]

Expected result:
  The focused tab visually moves right of the last tab. moveTabForward() moves
  the tab to the right of the next visible tab. Visual tab order changes.
    all tabs:
      [about:blank(1)(hidden)][about:blank(2)][about:home(selected,focused)]
    visible tabs:
      [about:blank(2)][about:home(selected,focused)]

Comment 7

[ithinc]

Attachment: patch v2 on top of bug 822068

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 695496 [details] [diff] [review]
patch v2 on top of bug 822068

>       <method name="moveTabForward">
>         <body>
>           <![CDATA[
>-            var tabPos = this.mCurrentTab._tPos;
>-            if (tabPos < this.browsers.length - 1)
>-              this.moveTabTo(this.mCurrentTab, tabPos + 1);
>+            let tab = this.mCurrentTab.nextSibling;
>+            while (tab && tab.hidden)
>+              tab = tab.nextSibling;
>+
>+            if (tab)
>+              this.moveTabTo(this.mCurrentTab, tab._tPos);

rename tab to nextTab

>       <method name="moveTabBackward">
>         <body>
>           <![CDATA[
>-            var tabPos = this.mCurrentTab._tPos;
>-            if (tabPos > 0)
>-              this.moveTabTo(this.mCurrentTab, tabPos - 1);
>+            let tab = this.mCurrentTab.previousSibling;
>+            while (tab && tab.hidden)
>+              tab = tab.previousSibling;
>+
>+            if (tab)
>+              this.moveTabTo(this.mCurrentTab, tab._tPos);

... and to previousTab here

Comment 9

[ithinc]

Attachment: patch for checkin

Comment 10

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b6cd9913f669

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b6cd9913f669