71832 - Add F3 shortcut for Find Again

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P3)

Description

[:deanis74]

Right now I can only use Ctrl+G as a shortcut for Search > Find Again.  In
almost all other applications, as well as 4.x, I can use F3.

Marking the OS as all because I'm assuming that all other platforms had this in 4.x.

Comment 1

[:deanis74]

Adding 4xp keyword.  Feel free to dupe this, but I searched for "f3" and
couldn't find anything.

Comment 2

[Peter "jag" Annema]

Well, F3 doesn't work on linux NS4.76 (alt+g does). I think this should be in
some platform* file, probably just the Windows one.

Comment 3

[:deanis74]

I'm for that.  There's a file somewhere that we add IE shortcut support, such as
F5 for refresh.  Perhaps we could stick it in there?

Comment 4

[Peter "jag" Annema]

I'm not sure, this sounds like something we want wherever we use the find
dialog, not just the browser.

Comment 5

[:deanis74]

Good point.

Comment 6

[Aaron Leventhal]

Putting this in the keyboard reference
www.mozilla.org/projects/ui/accessibility/mozkeylist.html

also the faq is important:
www.mozilla.org/projects/ui/accessibility/mozkeyintro.html

Comment 7

[:deanis74]

I tried to add this line to navigatorOverlay.xul and it didn't work.  Any idea why?

<key id="key_findAgainF3" keycode="VK_F3" command="Browser:FindAgain"/>

Comment 8

[:deanis74]

Attachment: I was using two different builds

Comment 9

[:deanis74]

The patch I attached adds F3 for Find Again in Navigator and Composer.  That's 
the only places that I could find such functionality.

1. It adds it for all platforms - is this really a bad thing?
2. I don't think the keys really need the id attribute, but I thought it
   wouldn't be a bad idea to add it.
3. Whoops, forgot to put my name in them.

Comment 10

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

Note that the other binding for find again in 4.x is Ctrl+G for find again. I 
don't think Mac uses function keys, so it would only use Cmd+F & Cmd+G. Adding 
support for F3 to mac will probably make pinkerton cry, file bugs against me, 
and whine on IRC until we all want to jump out a window... ;) 

Comment 11

[:deanis74]

I assume that Cmd+G is what's currently there for Mac right now, as that's 
what's there for Windows.  And if pinkerton cries, who cares?  Just jokin'.  So 
no for Mac.  What about Linux?

Comment 12

[Peter "jag" Annema]

No on Linux.

Just add it to Windows only (and probably OS/2).

Comment 13

[:deanis74]

OK, I've stuck this in platformNavigationBindings.xul for Windows.  Will be easy 
to do for OS/2 as well.

I'm having a few problems with the editor, though.  It would seem that the 
natural place to stick this would be in platformEditorBindings.xul.  But it 
doesn't look like this is included anywhere, and I'm not sure if it's been kept 
current.  The keyset id was incorrect - "editorKeySet" instead of "editorKeys".  
But Blake just added that with a recent check-in, so it's probably nothing big.

So if I add platformEditorBindings.xul as an overlay to editor.xul and change 
the keyset id, everything works splendidly.  Errr... maybe I'll just attach a 
patch.

...Oh man, I'm coding XUL now.  I should really stick to C.

Comment 14

[:deanis74]

Attachment: This works well for me, for both browser and editor.

Comment 15

[:deanis74]

cc: Blake.  Maybe he knows the answer to my query about 
platformEditorBindings.xul.

Comment 16

[Kathleen :Brade]

The editor file you changed is NOT the correct keybinding file, please don't add 
it to editor.xul.

Change this file:  mozilla/content/xbl/builtin/win/platformHTMLBindings.xml

Comment 17

[:deanis74]

Oh, cool.  That makes it really easy (I think).  I'll look at this early next week.

Comment 18

[:deanis74]

Attachment: New patch, using platformHTMLBindings.xul

Comment 19

[:deanis74]

OK, this patch stuffs both F3 keys into platformHTMLBindings.xul.  I'm not too 
sure about using "oncommand=BrowserFindAgain()" in the <browser> binding instead 
of "command=...", but as far as a XUL neophyte as I can tell, I'd have to define 
a command to handle that (see 
http://lxr.mozilla.org/seamonkey/source/xpfe/browser/resources/content/navigator
Overlay.xul#136).

Let me know if this patch jives.  If it does, I'll do the same thing for OS/2.

Comment 20

[Alec Flett]

hmm... I think you should define the command rather than embedding JS in the XBL

Comment 21

[:deanis74]

Maybe I spoke too soon.  I can't seem to define a command in the file.  I've 
tried sticking this code chunk nearly everywhere in that file, with no luck.

  <commandset id="commands">
    <command id="Browser:FindAgain" oncommand="BrowserFindAgain();"/>
  </commandset>

My F3 handling code is simple:
      <handler event="keypress" keycode="VK_F3" command="Browser:FindAgain"/>

Comment 22

[Aaron Leventhal]

-> Dean, will you take this?

Comment 23

[:deanis74]

Blake or Jag, do you have any ideas on this?  See my comments from 04-23 and 04-24.

Comment 24

[jag (Peter Annema)]

I'd say just use the oncommand version for now, or file a bug on making
Navigator use a command controller and mark this depend on it.

What should happen when a user presses F3 without having searched before? Right
now Ctrl+G says "can't find the text you entered". I think it should do the
equivalent of Ctrl+F. (That's what notepad does).

On Mac, IE disables command+G (like NS 4 does), on Windows, IE doesn't support
Cltr+G, and F3 opens a search sidebar.

Comment 25

[:deanis74]

Well, if you're fine with the oncommand version then attachment 31966 [details] [diff] [review] is ready
for review.

As for what F3 should do when a Find hasn't been performed, I agree.  But that
should be for any of F3, Ctrl+G, or Search > Find Again, so I'm guessing it
should be filed as a new bug about the find again functionality in Browser and
Composer.

Comment 26

[Jerome Lacoste]

bug 96810 was not considered a duplicate of this one saying that F3 is a windows
only thing. To be consistent, shouldn't this bug report be set to PC/windows only ?
In the other case, the comment on byg 96810 is erroneous (or there's something I
missed)

Comment 27

[Kathleen :Brade]

setting to Windows-only
hopefully a fix here will coordinate and also fix bug #96810 (and vice versa)

Comment 28

[:deanis74]

I'm confused.  There's a potential, windows-only patch here already.  For the
discussions in bug 96810, that one doesn't sound like a bug at all.

Comment 29

[:deanis74]

jag, if you want to r= this, I'll open a new bug on the
what-to-do-if-there's-no-find-text-when-finding-again issue.

Comment 30

[jag (Peter Annema)]

r=jag

Comment 31

[:deanis74]

Filed new bug 100760 - "Find Again should display Find dialog if no find string
is specified".  See also bug 65589 - "'Find Again' should be disabled if no find
string is specified".  I like the idea in 100760 better.

Blake, how 'bout an sr=?

Comment 32

[:deanis74]

Simple and low-risk fix.  -->0.9.8

Comment 33

[:deanis74]

Blake, can you take a second to sr= this?  It won't hurt, I promise!

Comment 34

[:deanis74]

Ping!  Still looking for an sr=.  It may be out-of-date with the tip, but I'll
update it once I get the sr=.

Comment 35

[:deanis74]

Forget it.  Over to the default owner.

Comment 36

[Blake Ross]

Attachment: patch

sorry, Dean, I suck.

Comment 37

[Blake Ross]

reassigning to myself.

Comment 38

[:deanis74]

Blake, what about F3 for editor?  I had this in my patch:

+      <handler event="keypress" keycode="VK_F3" command="cmd_findNext"/>

Comment 39

[Blake Ross]

Okay, but we'll also need it for mailnews too, and that won't work for this. 
This is really all a hack, we keep overloading these platform-specific files
because no one feels like creating new ones ;)  So in the spirit of that
tradition, I'm wondering if F3 is currently reserved for anything on other
platforms, or is well-known to do something on other platforms.  It would be
much easier if we could just add this shortcut xp, but if we have to delve into
the exciting wide world of overlays, we will.

Comment 40

[Kathleen :Brade]

please keep this as platform-specific keybinding
thanks!

Comment 41

[jag (Peter Annema)]

F3 has no place on linux or Mac as a "find again" binding.

Comment 42

[:deanis74]

Seriously now, 1.1?  Fine, give it back to me.  I'll update my patches and email
everyone I can to get this in before 1.0.

Comment 43

[Blake Ross]

Then here.  I've been told this isn't a priority and not to spend more than a
few minutes on it, which I'd have to if we resorted to overlays.

Comment 44

[Blake Ross]

Actually, mailnews uses <browser/> doesn't it? So I think Dean's patch is fine.
sr=me

Comment 45

[:deanis74]

Actually, I'm going to update the patch tonight.  I think I should be able to
use command=Browser:FindAgain instead of oncommand=BrowserFindAgain().

Comment 46

[Blake Ross]

I don't see how you can...

Comment 47

[:deanis74]

Oh, I see.  Your patch uses platformNavigationBindings while mine uses
platformHTMLBindings.  Which is the correct file?

Comment 48

[Blake Ross]

platformNavigationBindings if you want to use the command and not have it work
in mail, or platformHTMLBindings if vice-versa ;)

I'm undecided on the right fix here.  I'm kind of thinking it's to use overlays
because it's lame that the key binding would only work if the browser is focused.

Comment 49

[:deanis74]

What about using platformNavigatorBindings.xul, platformEditorBindings.xul, and
platformMailnewsOverlay.xul?

Comment 50

[:deanis74]

Well, I'm not sure any more where Blake wanted this to go.  Since I have very
little skill with overlays, back to Blake.... with love of course.

Comment 51

[R.K.Aa.]

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

Comment 52

[Don J. Rude]

I understand and agree that this isn't a major bug, in fact I personally see it
as an enhancement. But it sure is one I'd like to see. When I'm sitting at a
Windows box I automatically reach for F3 for find-in-doc.

There is a long history here and all it seems to need is sr= ? Or is there some
other work that really needs to be done?

Comment 53

[R.K.Aa.]

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

Comment 54

[Dan LeGate]

Hmmm... these comments go back so far that it's a bit confusing.

I'm seeing a lot about platformHTMLBindings.xul, but I'm not sure exactly how to
implement this (if it's even 'implementable' at this time).  Where to put it and
what to put in it is what I'm not sure about.  Do I include *all* the contents
of the 'patch' page that loads from the above link?

I'm not seeing the directories that are mentioned, so if they don't exist, do I
create them?

Where is "/cvsroot/mozilla..." or its equivalent on a Windows system?

Any help is greatly appreciated.  Thanks.

Dan
[e-mail]

Comment 55

[Itamar]

Just tested the patch and woks fine for me only in the browser (not in mail or
composer).
I have:
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.0.0+) Gecko/20020605

Now Dan
this is I tested this.
I've seen that the patch was made to file platformNavigationBindings.xul
This file is placed in one of the jar files but which one?
Jar files are basicly extended zip files.
so if you want to know in which jar file this file resides you can use windows
find tool.
Just search the mozilla.org folder and serach for a file that contains the text
"platformNavigationBindings.xul" - comm.jar seems to be the one.
Now just change the file ext to zip and unzip it.
Search for "platformNavigationBindings.xul" agin and open the file.
Add the patched line to the file and re zip it
(use http://www.ctan.org/tex-archive/tools/zip/info-zip/Zip.html
for some reason my zip util did somthing that mozilla didn't "like" and mozilla
woludn't start.)
Rename the file to comm.jar and replace the old one (always backup the old file).
Retsart mozilla - and TaDa - F3 works :-)
I hope this will get r= nad sr= soon since I realy like F3 as find again.

Comment 56

[Doron Rosenberg (IBM)]

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

Comment 57

[Aaron Leventhal]

-> Andrew

Comment 58

[andreww]

Ok some questions:

1) I assume this needs to work everywhere "find" works. And this would 
be mail, browser, and composer I assume. Are there other locations?

2) Given (1) above, which overlays need this patch?  If it is done in 
platformNavigationBindings.xul - what windows will this not work with? 

Comment 59

[[e-mail]]

The platform files:
platformNavigationBindings.xul - navigator & help overlay (name is wrong)
platformCommunicatorOverlay.xul - Used by platformBrowserBindings.xul
platformBrowserBindings.xul - Used when browser (including messenger) has focus
platformEditorBindings.xul - Used when editor (including composer) has focus
platformHTMLBindings.xml - More bindings for browser and editor!
platformMailnewsOverlay.xul - Used by messenger
platformMailOverlay.xul - Used by addressbook

Have I missed any?

Comment 60

[andreww]

ok, patch forthcoming!

Comment 61

[Aaron Leventhal]

Taking

Comment 62

[Aaron Leventhal]

checked in

Comment 63

[Bamm Gabriana]

Nominating for 1.0.x branch, I want this for our distro.

Comment 64

[:deanis74]

Aaron, what did you check in?

Comment 65

[Aaron Leventhal]

Sorry for the lack of clarity, this got fixed with the checkin for bug 167921

Comment 66

[sairuh (rarely reading bugmail)]

vrfy'd fixed using 2002.09.26.08 mozilla trunk builds.

on linux rh7.2 and win2k: F3 will find again and shift-F3 will find previous.

on mac 10.1.5, where the function keys don't conflict (eg, on my machine): F3
will find again and shift-F3 will find previous. (if they do conflict, cmd-G and
cmd-shift-G would work fine for find again/previous, respectively.)

Comment 67

[:deanis74]

This works in all windows: Composer, Browser, View Source, Help, and probably
one or two more that I missed.