Closed Bug 1016039 Opened 10 years ago Closed 10 years ago

"TypeError: candidate is undefined" when switching tab group (panorama) by keyboard shortcut

Categories

(Firefox :: Tabbed Browser, defect)

Product:
Firefox
Component:
Tabbed Browser
Version:
29 Branch
Platform:
x86_64
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 32
Tracking Flags:
Tracking Status
firefox29 --- wontfix
firefox30 --- wontfix
firefox31 + verified
firefox32 --- verified

People

(Reporter: yuki, Assigned: michael)

References

Details

(Keywords: regression, Whiteboard: [good first bug][mentor=dao][lang=js])

Attachments

(1 file)

Patch
744 bytes, patch
dao
: review+
Sylvestre
: approval-mozilla-aurora+
Details | Diff | Splinter Review 
The shortcut "Ctrl-Shift-`" switches the panorama. However, if the mouse cursor is on any tab, it fails to update visibility of tabs. As the result, broken group appears.

Steps to reproduce:
 1. Open a new browser window.
 2. Open the browser console by Ctrl-Shift-J.
 3. Open three tabs: "about:blank?1", "about:blank?2", and "about:blank?3".
 4. Enter to the Panorama view by Ctrl-Shift-E.
 5. Create two groups:
     A - "about:blank?1"
     B - "about:blank?2" and "about:blank?3"
 6. Click "about:blank?1" thumbnail to enter the group A.
 7. Now there is only one visible tab.
    Then, move your mouse cursor on the tab.
 8. Switch group by Ctrl-Shift-`.

Actual result:
  I'm in the group B, but there is only one visible tab "about:blank?2".
  The secondary tab in the group "about:blank?3" is still hidden.
  And, the error "TypeError: candidate is undefined tabbrowser.xml:4725"
  appears in the browser console.

Expected result:
  I'm in the group B and there are two visible tabs "about:blank?2" and
  "about:blank?3". There is no error in the browser console.

Tested environment:
 * Windows 7 Home Premium
 * Firefox 29.0.1 and Nightly 32.0a1 (2014-05-26)
 * With no addon

The error is always reported from the section in tabbrowser.xml:

>      <method name="_mouseenter">
>        <body><![CDATA[
>          if (this.closing)
>            return;
>
>          let tabContainer = this.parentNode;
>          let visibleTabs = tabContainer.tabbrowser.visibleTabs;
>          let tabIndex = visibleTabs.indexOf(this);
>          if (tabIndex == 0) {
>            tabContainer._beforeHoveredTab = null;
>          } else {
>            let candidate = visibleTabs[tabIndex - 1];
>            if (!candidate.selected) {  // <= this line.
Seems like _mouseenter should return early if the tab is hidden (this.hidden) just like it does for this.closing.
Whiteboard: [good first bug][mentor=dao][lang=js]
Attached patch Patch — — Splinter Review 
Updating _mouseenter to return early for hidden as well as closed tabs seems to fix this problem.
Attachment #8432113 - Flags: review?(dao)
Comment on attachment 8432113 [details] [diff] [review]
Patch

Thanks!
Attachment #8432113 - Flags: review?(dao) → review+
Assignee: nobody → michael
Comment on attachment 8432113 [details] [diff] [review]
Patch

Here are the try server results:

https://tbpl.mozilla.org/?tree=Try&rev=4d5bc721cf23
Attachment #8432113 - Flags: checkin?(dao)
Attachment #8432113 - Flags: checkin?(dao)
https://hg.mozilla.org/mozilla-central/rev/773efcc4ed6b
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 32
I think this should be fixed in Aurora31.0a2 which will be next ESR.
status-firefox29: --- → affected
status-firefox30: --- → affected
status-firefox31: --- → affected
status-firefox32: --- → fixed
tracking-firefox31: --- → ?
Flags: needinfo?(michael)
Blocks: 879597
Keywords: regression
Comment on attachment 8432113 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 879597

User impact if declined: see "Steps to reproduce" and "Actual result" in comment 0 (there's not just the error, but tabs erroneously remain hidden)

Testing completed (on m-c, etc.): on m-c

Risk to taking this patch (and alternatives if risky): very low

String or IDL/UUID changes made by this patch: none
Attachment #8432113 - Flags: approval-mozilla-aurora?
Flags: needinfo?(michael)
Attachment #8432113 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Reproduced in Nightly 2014-06-01.
Verified fixed 32.0a1 (2014-06-09), win 7 x64.
Status: RESOLVED → VERIFIED
status-firefox32: fixed → verified
Reproduced with Nightly 2014-05-26 on Mac OS X 10.9.2.
Verified as fixed with latest Aurora 31.0a2 (Build ID: 20140609004001) on Windows 7 x64, Mac OS X 10.9.2 and Ubuntu 14.04 x32.
status-firefox31: fixed → verified
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: