Closed Bug 463299 Opened 12 years ago Closed 12 years ago

Hitting Esc when text was entered in the search box shouldn't close the all tabs / ctrl-tab panel

Categories

(Firefox :: Tabbed Browser, defect)

x86
Windows Vista
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 3.1b2

People

(Reporter: whimboo, Assigned: dao)

References

Details

(Keywords: access, verified1.9.1)

Attachments

(1 file)

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081105 Minefield/3.1b2pre

As we already do in all other dialogs, the panel shouldn't be closed when text is entered in the search box and you hit Esc. In such a case the text only has to be cleared.

Steps:
1. Open at least 3 tabs
2. Open the "all tabs" or ctrl-tab panel
3. Enter some text inside the search box
4. Hit Esc

With step 4 the panel shouldn't be closed. Instead only the text should be cleared.
Flags: in-litmus?
Depends on: 463227
Attached patch patchSplinter Review
Assignee: nobody → dao
Attachment #346579 - Flags: review?(gavin.sharp)
Status: NEW → ASSIGNED
Comment on attachment 346579 [details] [diff] [review]
patch

>     // advance selection and close panel
>+    EventUtils.synthesizeKey("VK_TAB", {});
>     EventUtils.synthesizeKey("VK_TAB", {});
>     EventUtils.synthesizeKey("VK_RETURN", {});
>     ok(!isOpen(),
>        "Enter key closes the panel");
>     is(gBrowser.tabContainer.selectedIndex, 1,
>        "Tab key advances the selection while the panel is sticky");

Initial focus is on the second preview, but clearing the search field sets focus to the first one. Thus the extra TAB to select the third preview.
Flags: blocking-firefox3.1?
Attachment #346579 - Flags: review?(gavin.sharp) → review+
A second ESC should, however, close it.
Flags: blocking-firefox3.1? → blocking-firefox3.1+
My knowledge in key handling isn't good enough. So let me ask a question. Shouldn't this be handled by the search widget itself? IMO it doesn't make sense to check it for each instance.
> Shouldn't this be handled by the search widget itself?

What exactly?
http://hg.mozilla.org/mozilla-central/rev/40d02210c6c5
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.1b2
(In reply to comment #5)
> > Shouldn't this be handled by the search widget itself?
> 
> What exactly?

The topic this bug is about. But to be precise: Shouldn't the search widget handle the ESC key event itself?

(In reply to comment #3)
> A second ESC should, however, close it.

Dao, don't we have a test for that right now? I cannot find it.
(In reply to comment #7)
> Shouldn't the search widget handle the ESC key event itself?

It does, but it can't prevent capturing event listeners from consuming the event as well.

> > A second ESC should, however, close it.
> 
> Dao, don't we have a test for that right now? I cannot find it.

There's none.
Verified with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081107 Minefield/3.1b2pre

After talking with Marcia on IRC it would be great to have an automated test as well for closing the panel with ESC (see comment 3). Resetting in-testsuite flag to indicate that we need one more test here.
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
Flags: in-testsuite+
Flags: in-litmus?
Lets carry over the in-testsuite flag to bug 460560 where it better belongs to.
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.