88739 - F1 on Win/*nix, Cmd+? on Mac key doesn't bring up help

Status: RESOLVED FIXED

(SeaMonkey :: Help Documentation, defect)

Description

[AriB]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.2+) Gecko/20010701
BuildID:    2001070108



Reproducible: Always
Steps to Reproduce:
1. Start up mozilla and wait for home page to finish loading
2. Press the function key F1 (standard Help key on Windows)


Actual Results:  Nothing

Expected Results:  Bring up Help Window

Comment 1

[Francisco León]

dupe of bug 55305 or bug 46226 ?

Comment 2

[AriB]

No. It could depend on 55305 but in order for this bug to be fixed, F1 must call
up Help Contents and should be listed in the menu just like NS 4.7x did.
46226 is about context sensitive help which is completely different than calling
Help Contents from anywhere

Comment 3

[Matthias Versen [:Matti]]

I can't find a dupe. marking new

Comment 4

[Ian Oeschger (gone)]

I am on this one. Just to figure things out, I've gotten a thing working where I 
have Ctrl+8 opening the help window (modeled after and stuck in the 
tasksOverlay stuff), but how do I bind the <F1> key? 

I guess I don't understand the relationship between this key and, say, the keys 
defined in:
http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/locale/en-US/keys.
properties

Is there a precedent in the UI for using this kind of key as opposed to the 
accel+somekey combo?

Comment 5

[Ian Oeschger (gone)]

Hello! Just got it:
<key id="key_help" keycode="VK_F1" command="toHelp();" />
Now just trying to figure out where I should put this. Have it now in 
tasksOverlay, though that set of files seems more particular to the Tasks menu 
itself. utilityOverlay?

Comment 6

[AriB]

F1 is the standard Windows help key. It's NS 4.7x too

Comment 7

[Ian Oeschger (gone)]

Attachment: adding f1 key to help

Comment 8

[rudman]

Ian, let's get this one in on the trunk (once you get an r and sr) and see if
it's OK---maybe Terri can help verify. Once it's deemed OK, we'll check it in on
the branch.

Adding nsBranch keyword.

Comment 9

[Ian Oeschger (gone)]

Adding blake for review: F1 launching help.

Comment 10

[Blake Ross]

Why isn't the menuitem using the command also?

Comment 11

[Ian Oeschger (gone)]

blake & walk84
Doesn't seem to work when I hook up the menu. It may be something to do with the 
id's being used, the possibility that there is some name stomping going on with 
something as basic as "help" or "key_help" or something, but for whatever 
reason, doing
<menuitem key="key_help" command="Help:open" ...> gets me nothing from the menu 
(though <F1> still works fine). Feel like OK'ing it anyway?

Comment 12

[Stephen Walker]

r=walk84

Comment 13

[Blake Ross]

It's because you're not including the command along with the menuitem (in 
the files that this overlay overlays onto)...that said, I don't see how the key 
works either...

Comment 14

[Ian Oeschger (gone)]

Attachment: updated F1 key in helpMenuOverlay.xul

Comment 15

[Ian Oeschger (gone)]

Updated patch so that the menuitem uses the command and key as it should. But 
this doesn't work (!), and it's because, as Blake said, I am not getting the 
command into the target file. How do I do this? 

In utilityOverlay.xul, there seem to be lots of individual keys and commands, 
but few keysets and commandsets, into which I had hoped to overlay the "key_set" 
and "Help:open" stuff I have in the overlay. In fact, one comment says the keys 
are "there for show" because the real bindings are defined in the 
platformBindings file.

Note that on linux, the menuitem seemed to pick up the key and display it as 
part of the label, where on win32 it doesn't show it (and also doesn't execute 
the command). hmmm...

Need help.

Comment 16

[Blake Ross]

You get to put <keyset id="helpKeys"/> and <commandset id="helpcommandset"/> in 
every in every file that helpMenuOverlay overlays onto.  Isn't XUL fun!?

(Note that, while the key has to be in the keyset, the command 
doesn't...however, if you're going to keep it in the commandset, please 
capitalize the 'C')

Have no fear, though, whenever Ben (or I, but it's assigned to him ;) gets 
around to doing the window and dialog bindings, this will be a built-in feature 
that you won't have to worry about.

Comment 17

[Ian Oeschger (gone)]

Wow, that's a bummer that you can't simply overlay keys and commands like you do 
the menuitems themselves. It looks like there are various ways to go about 
getting them put into the target files. I will attach a patch of how I hope to 
do this in one of the components, and with this sort of addition to the 
contents.rdf file in the help dir that registers the overlay:

  <!-- Declare overlay points used in this package -->
  <RDF:Seq about="urn:mozilla:overlays">
    <RDF:li resource="chrome://communicator/content/utilityOverlay.xul"/>
    <RDF:li resource="chrome://messenger/content/messenger.xul"/>
    <RDF:li resource="chrome://composer/content/editor.xul"/>
  </RDF:Seq>

  <RDF:Seq about="chrome://composer/content/editor.xul">
    <RDF:li>chrome://help/content/helpMenuOverlay.xul</RDF:li>
  </RDF:Seq>
  ...etc..

Blake, when you have a moment, let me know if this is what you meant--if this is 
good form. Should I explicitly <?xul-overlay> the help file instead? And who 
else should get this besides the browser, the composer, and the messenger? 
addressbook? Thanks a lot

Comment 18

[Ian Oeschger (gone)]

Attachment: example of getting F1 hooked up in composer

Comment 19

[Ian Oeschger (gone)]

Attachment: A real patch for adding F1 to addrbook, editor, nav, mailnews

Comment 20

[Kathleen :Brade]

Note that this is a platform-specific bug.  On Mac, F1 is typically defined as 
Undo but a user can reset it.  The fix for this bug should be platform specific.

While we're at it, why doesn't the Help key on my Macintosh keyboard set to bring 
up help?  Solaris users also have a help key.

Comment 21

[Ian Oeschger (gone)]

Updating milestone to 0.9.4

Comment 22

[Jaime Rodriguez, Jr.]

Ian - Is this a stop ship for 0.9.4. If it is, please have your manager mark
this as nsbranch+.

Comment 23

[Ian Oeschger (gone)]

adding nsbranch+ on rudman's behalf (and per triage with him before his
vacation). I have the fix for this mostly in hand (viz the attachments here),
and it's an integral feature of the help system.

Comment 24

[Kathleen :Brade]

Regarding the proposed patch above, I don't agree that a Windows-specific 
keybinding should be added in a general location.  I would prefer to see a 
Windows-specific fix.

Comment 25

[Ian Oeschger (gone)]

Kathy
I have been poking around for a good platform-specific DTD for doing key
bindings (e.g. keycode=VK_F1 vs. keycode=mac help button) and not finding
anything. This seems like something we should already have done, and I realize
it's sort of late in the game to entertain changes of this nature, but do you
think we could cook up a new DTD for doing this sort of platform key binding
stuff. Like a platform_keys.dtd in global/locale?

I could imagine this being useful in a number of other cases besides the help
system, which needs it pretty bad. 

cc'ing jag

Comment 26

[Ian Oeschger (gone)]

Aaron
I was thinking about doing platform-specific keys.properties, and I understand
you are the one to talk to about this. Help needs F1 on windows but not on mac,
where I gather F1 gets used for other things. It seems like there would be some
other cases for using this platform split. 

Can you let me know whether this is doable and a good idea? Could you help me
push this through if it is something that would be of value?
Thanks, i.

Comment 27

[Asa Dotzler [:asa]]

0.9.4 is out the door.

Comment 28

[Aaron Leventhal]

Ian, use Cmd+? for contextual help on Mac, F1 on all other platforms.

Comment 29

[timeless]

Also support the help key on mac and sparc keyboards that have them :-).  See 
some bug assigned to me for info about the sparc stuff.

Comment 30

[Jaime Rodriguez, Jr.]

Adding correctness Status Whiteboard, correct/expected behavior does not occur.

Comment 31

[selmer (gone)]

Why is this a stop-ship for this release?  6.1 went out without this fix.

No reviews, patch is not complete (doesn't address platform issues.)

PDT- unless and until these issues can be resolved.

Comment 32

[Ian Oeschger (gone)]

Futuring to 0.9.6

Comment 33

[Aaron Leventhal]

NOTE: This is very useful, but only if it brings up contextual help for the
currently focused item.

Comment 34

[rudman]

Resetting milestone.

Comment 35

[Ian Oeschger (gone)]

Futuring to 1.0

Comment 36

[Jaime Rodriguez, Jr.]

will this be fixed for 1.0? i was confused by the "futuring" comment.

Comment 37

[rudman]

Ian, I don't remember if this is In or Out for this release....Your call.

Comment 38

[Ian Oeschger (gone)]

I think it's out. Unless I can get some more help. It turns out that there are
some issues with using the F1 key on the mac, and we don't yet have any good way
of handling this for different platforms, where the help key could be mapped to
F1 on unix and windows but something else on the mac

Comment 39

[timeless]

we should have a good way. we do have platform*bindings files.

the big question might be do we want to bind it to VK_HELP and then have 
bindings which control what maps to that, or do we want to directly go from 
VK_F1 to show_help()

Comment 40

[:deanis74]

That would be platformDialogOverlay.xul for dialogs, right?  But I'm unsure of
what to use for normal windows.  There's platformHTMLBindings, but when talking
about bug 71832 Blake told me that would only work if <browser> had focus.

As for the mapping, I think that our standard lately has been to register a
command and then map a key to that command, instead of using oncommand.  I don't
think we need the intermediate step of going to VK_HELP, do we?

Comment 41

[timeless]

for mac we kind of do because the key is labeled help.  on this (mac) usb 
keyboard it's also the insert button.  if I saw VK_INSERT bound to show_help() 
i'd probably scream.

Comment 42

[Kathleen :Brade]

timeless--currently the mac widget code maps the "ins/help" key to VK_INSERT

http://lxr.mozilla.org/seamonkey/source/widget/src/mac/nsMacEventHandler.cpp#820

We could easily change it to map it to VK_HELP instead.

Comment 43

[Jaime Rodriguez, Jr.]

removing PDT grafitti. 

looks like we got a patch, does this have a chance of making 099?

Comment 44

[Ian Oeschger (gone)]

I'm afraid this patch doesn't account for the platform specific stuff we still
need to do. I created this thinking we could use <F1> all the way around. Need
to make another one.

Comment 45

[timeless]

Filed bug 124393 VK_HELP. Feel free to fix the title etc.

Ian: i'd suggest you write a fix that relies on VK_HELP, ignore the other possibilities.

Comment 46

[Kathleen :Brade]

Comment on attachment 41079 [details] [diff] [review]
A real patch for adding F1 to addrbook, editor, nav, mailnews

marking patch so casual readers of this bug are aware that this patch isn't the
final patch

The problem is that this should be platform-specific:

+  <key id="key_help"	keycode="VK_F1"      command="cmd_help"  />

Comment 47

[Ian Oeschger (gone)]

OK, I'll go to town on a new patch and assume that 124393 will be fixed, since
that is the best way to make this work on different platforms, but the
dependency is a real one.

Comment 48

[scottputterman]

Moving Netscape owned 0.9.9 and 1.0 bugs that don't have an nsbeta1, nsbeta1+,
topembed, topembed+, Mozilla0.9.9+ or Mozilla1.0+ keyword.  Please send any
questions or feedback about this to [e-mail].  You can search for
"Moving bugs not scheduled for a project" to quickly delete this bugmail.

Comment 49

[timeless]

the blocker is nearly fixed...

Comment 50

[Blake Ross]

Ian, I fixed this while doing the menu restructuring, however I just noticed two
problems:

1. It's only fixed in commercial.
2. It hardcodes VK_F1; I see now it should be VK_HELP.

I think we should just go ahead and try to change it to VK_HELP for machv.  The
code is at
http://lxr/commercial/source/xpfe/communicator/resources/content/communicatorOverlay.xul#99

Comment 51

[Kathleen :Brade]

argh! who reviewed that?? That's not in a platform-specific file!
please take it out asap!

Comment 52

[Blake Ross]

I thought VK_HELP was supposed to do the right thing?

Comment 53

[Ian Oeschger (gone)]

I think brade is reacting to the VK_F1 stuff, not the change you suggest, blake.
Can we go ahead and make a patch for this?

Comment 54

[Blake Ross]

That's what I'm asking... will VK_HELP do the right thing on each platform, 
i.e., can I just make a patch of VK_F1 -> VK_HELP?  Or are platform-specific 
files needed?

Comment 55

[Kathleen :Brade]

Using this query:
http://lxr.mozilla.org/seamonkey/search?string=VK_HELP

it appears that only Mac builds are able to send VK_HELP key codes.

There probably should be a bug for each platform to handle this (Solaris (bug
48322 or bug 13168?), Windows, Linux, etc.).  Some platforms may already have that.

Long ago, I would have guessed that this bug was intended to do the
platform-specific work for Windows since F1 is a windows-specific keybinding.

At this point, I think the easiest thing is probably to do the platform-specific
overlays (unless you are comfortable working in the Windows key event code).

Comment 56

[Aaron Leventhal]

I did not know this before:

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnacc/html/ATG_KeyboardShortcuts.asp

Says that F1 should bring application help.
and that shift+F1 should bring up contextual help.

Comment 57

[Aaron Leventhal]

Changing summary, to reflect OS standards. Sorry I didn't have this correct before.

Comment 58

[:deanis74]

You just changed the meaning of this bug for Windows.  Shift+F1 on Windows
should display a tooltip for help.  It's like using the question mark in a
dialog and clicking an object.  F1 should still display context-sensitive help,
being the help window opened to a topic related to the window and/or function
you're using.

I would think this would be similar on *nix, too.

Please change the summary back to the original, "F1 key doesn't bring up
contextual help".

Comment 59

[Aaron Leventhal]

Alright, changing back as Dean says.
I'm a bit confused by the descriptions of what the keys should do on the MS page.

Comment 60

[:deanis74]

Shift+F1 displays a little tooltip.  Right-click on your desktop, select
Properties, then press Shift+F1.

Comment 61

[Ian Oeschger (gone)]

-->blake
Blake, I think you just did some work on this?

Comment 62

[R.K.Aa.]

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

Comment 63

[Peter Trudelle]

nsbeta1- per nav triage team

Comment 64

[Matthias Versen [:Matti]]

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

Comment 65

[R.K.Aa.]

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

Comment 66

[R.J. Keller]

Moving to default owners.

Comment 67

[David Balažic]

Ping...

We are at 1.7b now. Will this ever be solved ?

Comment 68

[Brion Vibber]

Attachment: Updated 2001-07-03 patch to current trunk

Seems to work partially; with the patch F1 brings up help from Navigator and
Composer windows, but not from for instance the Bookmark Manager or the
Preferences dialog.

Mac needs an overlay to use Cmd+? instead of F1 however, and to show the
accelerator in the menu. Haven't tested on other platforms.

Comment 69

[R.J. Keller]

> +  <key id="key_help"   keycode="VK_F1"      command="cmd_help"  />

You should use VK_HELP instead of VK_F1 for the help key.

Thanks for this patch! I'll compile this into a build and see if I can't get it
to work in Bookmark Manager (and other places, probably).

Comment 70

[Jeff Walden (gone starting June 8)]

Attachment: Testcase - VK_HELP is *useless*

> You should use VK_HELP instead of VK_F1 for the help key.

VK_HELP is *completely* dysfunctional (on non-Mac platforms, from what I can
gather).  See <http://lxr.mozilla.org/mozilla/search?string=VK_HELP>.  See also
the attached testcase; VK_HELP will not work while VK_F2 will, even though the
implementation syntaxes are exactly the same.  (Sorry for the unusual size of
the XUL window; my grasp of XUL is somewhat minimal.)

I don't know what needs to be done to hook up VK_HELP for real on all platforms
(if it is indeed hooked up on the Mac).  In the interim I say go for a
preprocessor #ifdef by platform.

(R.J., note that this also applies to the equivalent Firefox Help bug:
<http://bugzilla.mozdev.org/show_bug.cgi?id=5409>.  With 0.9/1.0 coming I say
use the hack, because there's no way VK_HELP will be fixed on the branch.)

Comment 71

[Brion Vibber]

(In reply to comment #70)
> I don't know what needs to be done to hook up VK_HELP for real on all platforms
> (if it is indeed hooked up on the Mac).  In the interim I say go for a
> preprocessor #ifdef by platform.

VK_HELP does not seem to be hooked up properly on the Mac: the test case does 
not trigger when I push Help. (Seamonkey build 2004052708, Mac OS X 10.3.4.)

Note that the "Help" key on the Mac doesn't actually bring up help in any application anyway, so far as I 
can tell. (In some apps it turns the cursor into a question mark, then beeps at you when you click 
something, but never produces any help.) Laptop keyboards don't even have the key... the proper 
access key for help in Mac OS X is Command+? (Command+Shift+/).

Comment 72

[ari haviv]

I hope this bug is fixed soon. F1 has been a windows standard since...forever 

Comment 73

[R.J. Keller]

Can someone tell me the Help key on the Mac? We need to get this wrapped up.
Firefox Help has had this for awhile.

Comment 74

[Stefan [:stefanh]]

(In reply to comment #73)
> Firefox Help has had this for awhile.

Not if you're on a mac...

Comment 75

[R.J. Keller]

(In reply to comment #74)
> (In reply to comment #73)
> > Firefox Help has had this for awhile.
> 
> Not if you're on a mac...

yup, that's why I want to know the mac keyboard shortcut for Help :).

Comment 76

[Stefan [:stefanh]]

> Note that the "Help" key on the Mac doesn't actually bring up help in any
application anyway, so far as I 
> can tell. (In some apps it turns the cursor into a question mark, then beeps
at you when you click 
> something, but never produces any help.) Laptop keyboards don't even have the
key... the proper 
> access key for help in Mac OS X is Command+? (Command+Shift+/).

Brion, in the applications I have tried Command+? (on my keyboard
Command+Shift++) the app help loads in the OS X Help Viewer. On the contrary,
Fetch and IE 5.2 loads its own help viewer when pressing the Help key. So, I
wonder if the Help key isn't a better alternative?

On the other hand, looking at
http://developer.apple.com/documentation/UserExperience/Conceptual/OSXHIGuidelines/http://developer.apple.com/documentation/UserExperience/Conceptual/OSXHIGuidelines/
Apple says: "the Help key (or Command-? or Command-/) invokes the application’s
help in Help Viewer".

Comment 77

[Stefan [:stefanh]]

(In reply to comment #76)

Correct link:

http://developer.apple.com/documentation/UserExperience/Conceptual/OSXHIGuidelines/

Comment 78

[R.J. Keller]

Attachment: Patch

My computer is a bit messed up, so I don't have a build system to test this
patch on.

Comment 79

[Brion Vibber]

(In reply to comment #76)
> Brion, in the applications I have tried Command+? (on my keyboard
> Command+Shift++) the app help loads in the OS X Help Viewer. On the contrary,
> Fetch and IE 5.2 loads its own help viewer when pressing the Help key. So, I
> wonder if the Help key isn't a better alternative?

Those are both very old applications, designed for the old Mac OS. IE hasn't been updated since 2001, 
when OS X was young. The only apps I regularly use that respond to the Help key are iTunes and 
FileMaker, and both also respond to command+? and list it as the shortcut key in the Help menu. Both 
also ran on the old Mac OS in the olden days, if I'm not mistaken.

Further, my PowerBook's built-in keyboard doesn't even *have* a help key.

If you want it to launch help in response to _both_ "Help" and command+?, that would probably be fine, 
but if you have to choose one it really needs to be command+?. Not doing so would be like declaring 
that "SysRq" will bring up Help on Windows because you're ReQuesting help from the SYStem. :)

Comment 80

[Brion Vibber]

(In reply to comment #78)
> My computer is a bit messed up, so I don't have a build system to test this
> patch on.

Doesn't seem to do anything on Mac OS X.

Comment 81

[[e-mail]]

Comment on attachment 154576 [details] [diff] [review]
Patch

keycode="?" is wrong; you probably need key="?" modifiers="accel" - also,
unless it would really hurt something, you might as well define both keys - and
can you put the keys in the existing keyset please?

Comment 82

[[e-mail]]

Comment on attachment 154576 [details] [diff] [review]
Patch

Oh wait, helpMenuOverlay... well, navKeys is still wrong, your irc suggestion
of tasksKeys might work or you could try just <keyset>

Comment 83

[[e-mail]]

Comment on attachment 154576 [details] [diff] [review]
Patch

Ah, having platform keys would be nice if the menu displayed the correct key.

Comment 84

[Felix Miata]

summary implies this is not a Win98/PC only bug

Comment 85

[Steffen Wilberg]

I made a testcase for VK_HELP:
http://bugzilla.mozilla.org/attachment.cgi?id=154806&action=view
You want "tasksKeys" indeed.

VK_HELP had the nasty side-effect of being triggered by almost any key. That's
bug 252750, which is already fixed on trunk. 

Also note that Cmd+? doesn't work, see bug 253680 comment 0.

Comment 86

[R.J. Keller]

--> default owners

Comment 87

[Stefan [:stefanh]]

Adjusting summary (we shouldn't bring up context help with those keys) and
taking bug. I'll give this a try once I've fixed bug 127704.

Comment 88

[Stefan [:stefanh]]

I'll start working on this next week.

Comment 89

[Stefan [:stefanh]]

Attachment: Make F1 on win/nix and Help/Cmd+? on mac open Help

Enable F1 on win/nix and Help/Cmd+? on mac. Only tested on mac... Part of the
patch based on Firefox code. The Cmd+? will not work properly on certain
locales (because of bug 280805 and/or the way the "native" mac menus work).
However, I do think it is worth implementing anyway. It's the standard-shortcut
for opening Help on mac. Also, there's the possibility of disabling the
shortcut.

Comment 90

[Robert Kaiser]

Comment on attachment 186109 [details] [diff] [review]
Make F1 on win/nix and Help/Cmd+? on mac open Help

this basically looks good to me, but as I feared I can't completely r+ this as
I don't know the <key> stuff well enough so that I can tell if it's fully
correct. I guess Neil can probably r+sr this though (once he's back).

Comment 91

[Kathleen :Brade]

Comment on attachment 186109 [details] [diff] [review]
Make F1 on win/nix and Help/Cmd+? on mac open Help

For '<key id="key_openHelpFrontend"', I don't see a command.  Is that
intentional?

What platforms/recent builds has this been tested on?

Comment 92

[Stefan [:stefanh]]

(In reply to comment #91)
> (From update of attachment 186109 [details] [diff] [review] [edit])
> For '<key id="key_openHelpFrontend"', I don't see a command.  Is that
> intentional?

Yes, that's intentional. key_openHelpFrontend is for the mac menu (Cmd+?). The
mapping is done different, so you need one command-less <key.../> for the menu
and one <key...> for the mapping of the command.

Shift+/ == ? on en-US keyboards. But pressing Cmd+Shift+/ will generate an
unshifted charcode (bug 280805), thus you need to map the openHelp() command to
Cmd+Shift+/ (the idea is taken from toolkit's baseMenuOverlay.xul).
> 
> What platforms/recent builds has this been tested on?

Only on mac (comment #89). It built and worked fine (en-US kb layout) ;)

Comment 93

[Kathleen :Brade]

Comment on attachment 186109 [details] [diff] [review]
Make F1 on win/nix and Help/Cmd+? on mac open Help

Thanks for the explanation; I missed it in the diff.
r=brade

Comment 94

[Stefan [:stefanh]]

Comment on attachment 186109 [details] [diff] [review]
Make F1 on win/nix and Help/Cmd+? on mac open Help

Neil suggested (via e-mail) a better way of doing this.  New patch is coming
soon...

Comment 95

[Stefan [:stefanh]]

Attachment: New version, no ifdefs

Here's a new version, following Neils suggestions (using overlays instead of
ifdefs). Also adding the Window menu to the View Source/View partial Source
window(s). Only tested on mac.

Brade, since you were so kind to volunteer last time, can you review this one
as well?

Comment 96

[Kathleen :Brade]

Comment on attachment 187405 [details] [diff] [review]
New version, no ifdefs

r=brade

Comment 97

[[e-mail]]

Comment on attachment 187405 [details] [diff] [review]
New version, no ifdefs

>+      <menu id="windowMenu"/>
>+
It occurs to me that most windows e.g. ChatZilla, DOM Inspector actually
include both the tasks and window menus, so I guess it would be more consistent
if you could do that here too.

>+    <key id="key_openHelp"
>+         key="&openHelpFrontend.key;"
>+         modifiers="&openHelpFrontend.modifiers;"/>
>+    <key key="&openHelpCmd.key;"
>+         modifiers="&openHelpCmd.modifiers;"
>+         command="cmd_openHelp"/>
>+    <key keycode="&openHelp2Cmd.key;"
>+         command="cmd_openHelp"/>
You might want to consider commenting on these a little. The .dtd's comment
isn't too great either.

>+  <!-- Tasks Keys -->
>+  <keyset id="tasksKeys">
>+    <key id="key_openHelp"
>+         keycode="&openHelpCmd.key;"/>
>+  </keyset>
You missed the command="cmd_openHelp" on this one. sr=me with this fixed.

Comment 98

[Stefan [:stefanh]]

Attachment: Final version, carrying over flags (Checked in)

Changed comments in the .dtd file, added comments in
mac/platformCommunicatorOverlay.xul and fixed the missing cmd_openHelp.
Carrying over review flags.

Requesting approval 1.8b3 for a SeaMonkey-only change. This will fix a
long-time missing feature: keyboard access to the Help Viewer.

Comment 99

[Benjamin Smedberg]

Comment on attachment 187556 [details] [diff] [review]
Final version, carrying over flags (Checked in)

Has this been tested to make sure it does not break Thunderbird?

Comment 100

[Stefan [:stefanh]]

Comment on attachment 187556 [details] [diff] [review]
Final version, carrying over flags (Checked in)

(In reply to comment #99)
> (From update of attachment 187556 [details] [diff] [review] [edit])
> Has this been tested to make sure it does not break Thunderbird?
> 

The critical file seems to be platformCommunicatorOverlay.xul, running
Thunderbird with its platforCommunicatorOverlay patched works fine. It looks
like it isn't used at all, there is no corresponding .dtd file in Thunderbirds
en-US.jar

Comment 101

[Ian Neal]

Comment on attachment 187556 [details] [diff] [review]
Final version, carrying over flags (Checked in)

Checking in extensions/help/resources/content/helpMenuOverlay.xul;
new revision: 1.14; previous revision: 1.13
extensions/help/resources/locale/en-US/shortcuts.xhtml;
new revision: 1.17; previous revision: 1.16
xpfe/browser/resources/content/viewSourceOverlay.xul;
new revision: 1.25; previous revision: 1.24
xpfe/communicator/resources/content/mac/platformCommunicatorOverlay.xul;
new revision: 1.13; previous revision: 1.12
xpfe/communicator/resources/content/unix/platformCommunicatorOverlay.xul;
new revision: 1.6; previous revision: 1.5
xpfe/communicator/resources/content/win/platformCommunicatorOverlay.xul;
new revision: 1.6; previous revision: 1.5
xpfe/communicator/resources/locale/en-US/mac/platformCommunicatorOverlay.dtd;
new revision: 1.7; previous revision: 1.6
xpfe/communicator/resources/locale/en-US/unix/platformCommunicatorOverlay.dtd;
new revision: 1.5; previous revision: 1.4
xpfe/communicator/resources/locale/en-US/win/platformCommunicatorOverlay.dtd;
new revision: 1.5; previous revision: 1.4
done