Closed Bug 598691 Opened 9 years ago Closed 9 years ago

Tab matches from secondary window are not switching on click

Categories

(Firefox Graveyard :: Panorama, defect, P2)

defect

Tracking

(blocking2.0 beta8+)

VERIFIED FIXED
Tracking Status
blocking2.0 --- beta8+

People

(Reporter: aaronmt, Assigned: seanedunn)

References

Details

Attachments

(1 file, 2 obsolete files)

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b7pre) Gecko/20100922 Firefox/4.0b7pre

With http://hg.mozilla.org/mozilla-central/rev/198709160138, bug 595236 landing, it seems like there is an issue present where one is unable to select a tab match result and have the browser switch on click.

STR:

1. Open a browser window, and head to www.yahoo.com
2. Open a new window, and enter Panorama
3. Do a search for 'Yahoo', see the result matched properly
4. Attempt to click the result

ER: Upon click, the browser will switch to the tab in it's corresponding window
AR: Screen flickers with the actual result but the browser does not change to the matched tab in it's corresponding window.
blocking2.0: --- → ?
Component: Tabbed Browser → TabCandy
QA Contact: tabbed.browser → tabcandy
Blocks: 597043
Priority: -- → P2
Assignee: nobody → seanedunn
blocking2.0: ? → beta8+
Attached patch v1 (obsolete) — Splinter Review
It looks like the focus was being canceled out by the event handling, which probably resulted in a focus on the current window. So, I used setTimeout to guarantee that it would focus after the current event is handled.
Attachment #479701 - Flags: feedback?(ian)
Comment on attachment 479701 [details] [diff] [review]
v1

Is it necessary for both key and click? 

Also, another approach may be to use: 

        event.stopPropagation();
        event.preventDefault();

If this works, it would be preferable to setTimeout.
Attachment #479701 - Flags: feedback?(ian) → feedback-
Status: NEW → ASSIGNED
Attached patch v2 (obsolete) — Splinter Review
I had tried event.stopPropagation(), but not event.preventDefault(). Turns out hideSearch() calls both of these but also focuses the window, and I resolved the bug by making sure it was called before we focus() the tabs, and not after.
Attachment #479701 - Attachment is obsolete: true
Attachment #480679 - Flags: feedback?(ian)
Comment on attachment 480679 [details] [diff] [review]
v2

Looks good. Note that bug 595560 also modifies code in the same area so there will be some merging necessary.
Attachment #480679 - Flags: feedback?(ian) → feedback+
Attachment #480679 - Flags: review?(dolske)
Duplicate of this bug: 602302
If Bug 595236 - Match Tabs From Other Windows in Panorama Search was a beta 7 blocker, why is this a beta 8 blocker? The feature doesn't work at all.
Attachment #480679 - Flags: review?(dolske) → review+
ready for landing
Attachment #480679 - Attachment is obsolete: true
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/62b1ccb93391
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
verified with nightly minefield builds of 20101021
Status: RESOLVED → VERIFIED
Tracy, I can still reproduce this with  Mozilla/5.0 (Windows NT 5.1; rv:2.0b8pre) Gecko/20101212 Firefox/4.0b8pre. If you can also confirm, or anyone, think we should reopen..
(In reply to comment #10)
> Tracy, I can still reproduce this with  Mozilla/5.0 (Windows NT 5.1;
> rv:2.0b8pre) Gecko/20101212 Firefox/4.0b8pre. If you can also confirm, or
> anyone, think we should reopen..

I just reconfirmed that this was fixed back in October.  It has since regressed again.  However, please file a new bug.
Flags: in-litmus?(twalker)
Test case added: https://litmus.mozilla.org/show_test.cgi?id=15106
Flags: in-litmus?(twalker) → in-litmus+
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.