112846 - Find in top window does not work when all text is selected

Status: VERIFIED FIXED

(SeaMonkey :: Composer, defect)

Description

[TucsonTester1]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:0.9.6+)
Gecko/20011126 Netscape6/6.1b1
BuildID:    2001112603



Reproducible: Always
Steps to Reproduce:
1. Launch Composer or Navigator (if Composer, enter some text into it)
2. Hold CTRL and press A
3. Hold CTRL and press F (make sure none of the search options are checked)
4. Search for something on the page

Actual Results:  When searching while all is highlighted, I receive the error
"The text you entered was not found"

It only locates the text if both wrap around and search backwards are selected
together.

Expected Results:  It should either find the text with none of the search
options checked, or do so with just wrap around selected.

Comment 1

[Kathleen :Brade]

Something sounds very wacky here.
Since I would guess that 80% of our users might try doing Find with everything
selected (and probably wouldn't have those other checkboxes set) at some time, I
think this should be EDITORBASE.

Syd--please adjust if you disagree.

Kin--is this a core bug?

Comment 2

[Syd Logan]

Confirmed. I totally agree Kathy, this is EDITORTBASE.

Comment 3

[Akkana Peck]

There's some question about what find should do if there's a selection.  Should it
1. find only in the selection?
2. find in the whole document but starting from the beginning of the selection?
3. ignore the selection and start from the beginning of the document (or the
last find position if this is a find again)?
I've heard several differing opinions on this, to the extent where I think it
might be worthwhile asking a wider audience.

Regardless of which solution we pick, the fix will live in my new find code, so
I'll take the bug.  

Comment 4

[kinmoz]

The reason this bug happens is because, the current find implementation starts 
the find *after* the current selection ... and in this case there is nothing 
after the selection, since the selection is the entire document. The reason find 
searches after the current selection is because you may be searching a 2nd time 
after you found a particular instance of a word already.

IMO, the new find backend code should know nothing about selection. That is it 
should only work on ranges ... you give it a range and it looks inside the range 
for the expression you gave it, and it returns a range containing it's 
findings, if any.

It's more flexible that way, and the app/component that is driving it, can 
decide these things. Also, if you remove selection out of the find backend 
equation, things like the spellchecker can use it to find and mark (with the 
spellchecker selection) all occurrences of a specific misspelled word, without 
touching the primary selection.

Item #1 in akk's list will need some new UI to support it. Also think about how 
you would handle the case where the current selection only contained the word 
you were searching for ... the 2nd find scenario.

Item #2 depends on which direction you are finding. I think you'd want to search 
from the end of the selection to the end of the doc when searching forward, and 
the beginning of the selection to the beginning of the doc when searching 
backwards.

Comment 5

[Simon Fraser [no longer active]]

Mac applications usually start Finding from the end of the current selection. If 
wrapping is on, and nothing was found up to the end of the document, they then 
wrap around, and start looking from the top, continuing into selected text. If 
the term is found inside the selected text, they'll change the selection to 
hilight the found term.

Comment 6

[Akkana Peck]

The new nsWebBrowserFind code implements Simon's suggestion.  That means that if
everything is selected, it will start from the end of the document, but if
wraparound is selected, it will wrap around and find the first match then.

It's not enabled by default -- that's tracked in bug 80805 -- but to test the
new find code before 80805 is fixed, do this:
user_pref("browser.new_find", true)

Comment 7

[michael wendell]

Verified in build 02-20.