249345 - find sticks on the same match

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[rbs]

Steps to reproduce:
1. visit mozilla.org
2. type mozilla in the search box
3. click to focus _before_ "search mozilla".
4. hit ctrl-F to bring the Find dialog
5. type mozilla in the Find dialog
6. close the Find dialog
7. hit F3 (i.e., Find Again) repeatedly

--> actual results: at some point, find sticks on the "mozilla" match that is on
"search mozilla". (If you don't see the bug, click again to focus _before_
"search mozilla", then hit F3 repeatedly.)

--> expected results: find should iterate over all matches.

Technical info:
---------------
I have debugged the problem and discovered that it is due to a difference of
focus state between the ESM and the focus controller. Specifically, here is what
happens.

1) When a match is initially found, the back-end of the find code calls the
ESM::MoveFocusToCaret() to update the focused element.

2) But ESM::MoveFocusToCaret() begins by checking that some its things haven't
changed, and thus takes a short-cut that leaves the focus controller with the
previous focused element -- which is the text <input> field in this case.

3) When Find Again (F3) comes, it uses that old element to detect the search
range. But at some point, the textfield has no selection, and so the find falls
back to "find around the document" rather that "from the current selection
point". Result: the same (first) match is found over and over again.

I will attach a patch. The patch simply checks if the local selection in the
textfield is empty. If so, it ignores it and relies on the document selection.

Comment 1

[rbs]

Attachment: patch

Comment 2

[rbs]

Comment on attachment 152093 [details] [diff] [review]
patch

asking for r/sr on this simple patch to ignore an empty selection in a text
control when appropriate.

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 152093 [details] [diff] [review]
patch

r+sr=jst

Comment 4

[Johnny Stenback (:jst)]

Assuming this lands w/o problems, we should make sure this gets into the aviary
branch as well.

Comment 5

[rbs]

Fixed on trunk.

[PS: The bug doesn't occur when the Find dialog is there because there is some
much changes in the ESM when flipping back and forth with the find dialog, thus
forcing the ESM::MoveFocusToCaret() not to take its early shortcut when updating
the focus state.]

Comment 6

[Johnny Stenback (:jst)]

rbs, wanna land this on the aviary 1.0 branch? Or let me know if you want me to
land it...

Comment 7

[rbs]

I don't have a handy aviary 1.0 branch right now. Go ahead with the landing.

Comment 8

[Johnny Stenback (:jst)]

Fixed on the aviary branch too.