63705 - Select contents of textfield in Find in this Page upon loading

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P3)

Description

[Blake Ross]

In cases where we remember the last find text and prefill it in the Find in 
this Page dialog, we should select all the text upon loading.  Attaching a 
patch which does this, fixes the context persistence (setting checked="" wasn't 
properly pre-checking the appropriate checkboxes), and does other clean-up.

Upon fixing the persistence, I noticed that the case sensitive checkbox was 
checked by default upon loading the dialog (even though I hadn't previously 
checked it).  After some digging, I found that mLastCaseSensitive was set to 
PR_TRUE in the nsFindComponent constructor, while the other two 
(mLastSearchBackwards and mLastWrapSearch) were set to PR_FALSE.  Simon, cvs 
blame says you did this back in 6/99 in rev. 1.19.  Any idea why 
mLastCaseSensitive should be PR_TRUE by default?

Comment 1

[Blake Ross]

.

Comment 2

[Blake Ross]

Attachment: [patch] select contents, fix persistence, other clean-up

Comment 3

[Blake Ross]

cc'ing alec for sr

Comment 4

[Blake Ross]

Attachment: [patch] (forgot something...)

Comment 5

[Blake Ross]

*** Bug 63729 has been marked as a duplicate of this bug. ***

Comment 6

[Matthew T (active 1999-2002)]

Doing this with code specific to the Find dialog seems unhealthy, as it implies
that every dialog will have to do the same thing -- and since this is expected
behavior for every dialog, they shouldn't really have to do it individually.

I think that (apart from the unrelated cleanup of the Find dialog, to make the
checkboxes persistent between searches) this should be a duplicate of bug 28583.
Whenever a text field takes focus, by any means other than clicking in it, its
contents should be selected.

Comment 7

[Blake Ross]

Matthew: I'll file a bug to remove it (and the similar Open Web Location code) 
once that bug is fixed.  Please resummarize bug 28583 to include every case 
except clicking, though (not just tabbing).

Comment 8

[Blake Ross]

cc'ing timeless for review

Comment 9

[timeless]

MPL not NPL. Please don't use a contributor section rely on cvs blame.

i'd prefer not to give my r= until the license is settled, but if someone says 
we use NPL then you can take this as my r=.

Comment 10

[Alec Flett]

mind doing a diff -bw so we can see what ACTUALLY changed? :)

Comment 11

[timeless]

heh, I had to use norton utilties ncompare in order to understand the changes.

Comment 12

[Blake Ross]

Attachment: [patch] -bwu

Comment 13

[Blake Ross]

Attachment: [patch] altogether now...

Comment 14

[Blake Ross]

alec, have time to sr this (crap) before you go vacationing?

Comment 15

[Blake Ross]

Just noticed the modeline says C++.  Fixed in my tree.

Comment 16

[Ben Goodger (use ben at mozilla dot org for email)]

a couple of things:





+    dump("*** ERROR ACCESSING FIND COMPONENT!\n");



is this ever likely to fail, and warrant this human readable message that an 

optimized build will never show unless it is run with a console? A user of a 

debug build (the only one likely to see any debug output as the result of failing 

to create the find component) may be more impressed with the actual js exception, 

which can be obtained by removing the try... catch



also, it looks like you're setting the checked attribute on checkboxes directly.. 

.checked is supposed to work... 

Comment 17

[Ben Goodger (use ben at mozilla dot org for email)]

with described changes, a=[e-mail]

Comment 18

[Blake Ross]

Fix checked in.

Comment 19

[Blake Ross]

Reopening for a little more cleanup.

Comment 20

[Blake Ross]

Attachment: [patch] remove dialog.cancel, use .value, only retrieve bundle if necessary

Comment 21

[Blake Ross]

jag/timeless, r=?

Comment 22

[Peter "jag" Annema]

r=jag

Comment 23

[Blake Ross]

cc'ing ben for a= on this last bit of cleanup that clearly doesn't belong in 
thsi bug ;)

Comment 24

[Blake Ross]

Rest of cleanup checked in.

Comment 25

[sairuh (rarely reading bugmail)]

vrfy fixed using 2001.05.29.0x comm bits on linux, winnt and mac.