319850 - Make F1/Cmd+? open the respective Help topic in prefwindows

Status: VERIFIED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Jeff Walden (gone starting June 8)]

I happened to be reading some of the posts on the MS Office blog for their next version when I happened to learn that F1 in some versions of Office opens Help with a context-provided topic:

http://blogs.msdn.com/jensenh/archive/2005/11/29/497861.aspx#498037

I immediately realized that we, unfortunately, do no such thing, even in the places where we *obviously* should (e.g., the prefs window and in dialogs with specified help topics).  Look for two patches to fix this within the next month-and-a-half on the XUL widgets which support helpTopic and helpURI attributes.  Why two?  It's an obvious feature for 2.0, and it's unlikely to be complicated in ways that might destabilize the widget code.

Comment 1

[Jeff Walden (gone starting June 8)]

Attachment: Patch - hook up on dialog and prefwindow elements

There's no harm in generating a "dialoghelp" event with no code to run (e.g., the About dialog); I don't see a real need for double-checking code if there's nothing to prevent.

The number of times the key information will be duplicated between browser and two separate uses in toolkit almost makes me think a new helpKeys.dtd or something might be in order.

Comment 2

[Asaf Romano (gone)]

Comment on attachment 210728 [details] [diff] [review]
Patch - hook up on dialog and prefwindow elements

> Index: mozilla/toolkit/locales/en-US/chrome/global/dialog.dtd
>===================================================================

let's make this globalKeys.dtd or something similar.

>Index: mozilla/toolkit/locales/en-US/chrome/global/preferences.dtd
>===================================================================

and alos include it in preferencex.xml.

>Index: mozilla/toolkit/content/widgets/dialog.xml
>===================================================================

>+
>+      <handler event="keypress"
>+#ifdef XP_MACOSX
>+               keycode="&openHelpMac.keyCode;"
>+#else
>+               keycode="&openHelp.keyCode;"
>+#endif
>+               phase="capturing">
>+        this._fireButtonEvent("help");
>+      </handler>
>+#ifdef XP_MACOSX
>+      <handler event="keypress" key="&openHelpMac2.key;" modifiers="&openHelpMac2.modifiers;" phase="capturing">
>+        this._fireButtonEvent("help");
>+      </handler>
>+#endif
>+
> #ifdef XP_MACOSX
>       <handler event="keypress" key="." modifiers="meta" phase="capturing" action="this.cancelDialog();"/>
> #else

 - Would "dialpghelp" listeners have to prevent the default event action if they also have a <xul:key> for it (e.g. on mac, from macBrowserOverlay)? Please document this here if so.
 - nit: merge the two #ifdef XP_MACOSX blocks
 

>Index: mozilla/toolkit/content/widgets/preferences.xml
>===================================================================

>     <handlers>
>       <handler event="dialoghelp">
>+      <![CDATA[
>         // currentPane is set in _selectPane for prefwindows which aren't childs.
>         // So if there's no currentPane, just use the first pane.
>         var pane = this.currentPane ? this.currentPane : this.preferencePanes[0];
>-        this.openHelp(pane.helpTopic, pane.getAttribute("helpURI"));
>+        if (pane.helpTopic && pane.hasAttribute("helpURI"))
>+          this.openHelp(pane.helpTopic, pane.getAttribute("helpURI"));

cache pane.helpTopic

>       <handler event="keypress" key="&windowClose.key;" modifiers="accel" phase="capturing">
>         if (this.instantApply)
>           window.close();
>       </handler>
>+      <handler event="keypress"
>+#ifdef XP_MACOSX
>+               keycode="&openHelpMac.keyCode;"
>+#else
>+               keycode="&openHelp.keyCode;"
>+#endif
>+               phase="capturing">
>+        this._fireEvent("dialoghelp", this);

Again, <xul:key>s fun. I'm pretty sure you'll have to prevent the default action in your own dialoghelp listener as well iff you've called |openHelp|.

Comment 3

[Steffen Wilberg]

Attachment: preferences only, updated to trunk

This is a bit simpler since bug 432112 made Mac just use Cmd+? instead of Cmd+Shift+?.
I went one step further and hardcoded modifiers="accel" for mac, since every locale uses "accel", well, apart from those few which translated "accel" to "engetela", "അക്സെല്‍", "முடுக்கப்படு", "gijimisa", or "ఎక్సెల్"...
(I'll do that to baseMenuOverlay.xul as well in bug 432380.)

I also skipped dialog.xml for now.
This works as expected in all prefpanes, tabs and sub-prefwindows.

(In reply to comment #2)
>  - Would "dialpghelp" listeners have to prevent the default event action if
> they also have a <xul:key> for it (e.g. on mac, from macBrowserOverlay)? Please
> document this here if so.
I don't quite understand what you mean here.
This code makes F1/Cmd+? trigger ondialoghelp.
preferences.xul and the sub-prefwindows like colors.xul have ondialoghelp="openPrefsHelp()" (since bug 423486).

Nothing would prevent them from having another key (say Accel+H) calling openPrefsHelp() or whatever directly.

Comment 4

[Asaf Romano (gone)]

Comment on attachment 341763 [details] [diff] [review]
preferences only, updated to trunk

r=mano

Comment 5

[Steffen Wilberg]

Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/f7e763f81f1f

Filed bug 470297 for dialogs.

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 341763 [details] [diff] [review]
preferences only, updated to trunk

a191=beltzner

Comment 7

[Steffen Wilberg]

Pushed to 1.9.1:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5b12da277083

Comment 8

[Charles Fenwick]

Verified.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090127 Shiretoko/3.1b3pre