266368 - Find bar is disabled when closed with "match case" focused

Status: RESOLVED FIXED

(Toolkit :: Find Toolbar, defect, P2)

Description

[Michael Clackler]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20041027 Firefox/1.0RC1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20041027 Firefox/1.0RC1

If the "Match case" option on the find bar has the focus when the bar closes,
the bar can not be opened using FAYT, /, or '.

Reproducible: Always
Steps to Reproduce:
1. Open the Find Bar
2. Select "Match case"
3. Close the find bar (esc or click "x") 

*** IMPORTANT *** Do not click anywhere after selecting "Match case" which would
change the focus from "Match case" (with the posible exception of closing the bar).

4.  Try and open the find bar using FAYT, /, or '

Actual Results:  
The find bar does not open

Expected Results:  
The find bar should open

Since "match case" is still focused, the shouldFastFind function disables the
keyboard inputs from opening the find bar.

There is some code in the closeFindBar function which removes the focus from the
findField when the bar closes to prevent this problem from hapening with the
findbar text box, but it does not remove the focus from the "match case" option.

Note: If you click anywhere on the page to change the focus from "match case",
the find bar will again open.

Comment 1

[Ian Macfarlane]

Confirmed (Firefox 1.0). In fact it can't be opened with ctrl-f either. You need
to change focus to something else (e.g. click within the main window) to get it
back again.

Comment 2

[Michael Clackler]

Attachment: patch to clear match case focus on toolbar close

Patch to fix the problem.

This clears the focus on the toolbar "match case" option when the toolbar
closes in the same manner the code cleared the focus on the find bar text
field.

Comment 3

[Michael Clackler]

requesting blocking-aviary1.1 since functionality is lost and patch exists

Comment 4

[Mike Connor [:mconnor]]

Comment on attachment 170629 [details] [diff] [review]
patch to clear match case focus on toolbar close

>+  var elt = document.commandDispatcher.focusedElement;
>+  if (elt) {
>+    var eltParent = elt.parentNode;
>+    if (window == ww.activeWindow &&
>+        ((eltParent.parentNode == findField) || (eltParent == findToolbar)))

the extra parens are bogus here, and we don't really need the extra var.  Just
use elt.parentNode directly.

   (elt.parentNode.parentNode == findField || elt.parentNode == findToolbar)

>+      _content.focus();

content.focus() here, please (_content was deprecated a while back, this should
have been fixed in that change but wasn't).

Comment 5

[u88484]

I can confirm this behavior so Changing Status to 'New' and also based on
mconnor reviewing the patch...guess he acknowledges the bug is present.

Comment 6

[Jaime Mitchell (use [e-mail] for email)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050621
Firefox/1.0+ ID:2005062113

I can still reproduce this (before changing match case you need to actually try
searching for something)

Looking at :
http://lxr.mozilla.org/mozilla/source/toolkit/components/typeaheadfind/content/findBar.js#324
The function closeFindBar() appears to have been completly changed so a new
solution is required.

Comment 7

[tyl2]

I haven't programmed for three years, so I'd be surprised if this works. But
I'll try anyway. Contrary to the comment #6, it seems function
delayedCloseFindBar() is now called when closeFindBar() is called. However, only 

document.commandDispatcher.focusedElement.parentNode.parentNode == findField

is checked, again forgetting that an element other than find-field may be
focused on FindToolbar. Therefore, line 380

var findToolbar = document.getElementById("FindToolbar");

should be moved to line 370 and line 372 should add a check for an element on
FindToolbar being focused, maybe something like

&& document.commandDispatcher.focusedElement.parentNode.parentNode == FindToolbar

but I don't know how many iterations of .parentNode there should be.

Sorry for the bugspam. I hope this is more useful than a spam.

Comment 8

[Asaf Romano (gone)]

Attachment: patch

Comment 9

[Asaf Romano (gone)]

Checking in findBar.js;
/cvsroot/mozilla/toolkit/components/typeaheadfind/content/findBar.js,v  <-- 
findBar.js
new revision: 1.21; previous revision: 1.20
done