260054 - add context sensitive Help

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[Steffen Wilberg]

I want to see "Help" buttons in a couple of dialogs, most important in Options.

The backend is already (still rather) in place, see
http://www.mozilla.org/projects/help-viewer/content_packs.html#launching_help_viewer

I removed the Help button in the password manager in bug 217147 (among other
things). It doesn't even have l10n imact as far as I can see.

Comment 1

[Steffen Wilberg]

Attachment: patch for the main Options dialog

Adding a Help button is not that hard :)
Getting it to work in the Options panel is a bit trickier since the button is
in pref.xul, but it has to open Help sections depending on the content shown in
an iframe in the right half of the dialog. So I'm adding the helpTopic to the
respective file and query that from nsPrefWindow.js.

R.J., comments? I guess you're somewhat more familiar with context-sensitive
Help!?

Comment 2

[R.J. Keller]

Sounds good to me. Usually I'd say this would be bad if help was to be not
built, but currently its not possible without hacking the makefiles. Definetely
a different issue than what arises with seamonkey. So I say sure IMO. As always,
I'd ask ben (and only ben) for review/approval. Sounds like a decision he should
make.

Comment 3

[Steffen Wilberg]

Comment on attachment 159273 [details] [diff] [review]
patch for the main Options dialog

found in nsPrefWindows.js:
* CHANGES MUST BE REVIEWED BY [e-mail]!!

That's you I guess!? Do you want me to change your email address as well?

Comment 4

[R.J. Keller]

(In reply to comment #3)
> (From update of attachment 159273 [details] [diff] [review])
> found in nsPrefWindows.js:
> * CHANGES MUST BE REVIEWED BY [e-mail]!!
> 
> That's you I guess!? Do you want me to change your email address as well?
> 

Not me :). I'd just say ben instead of a email address. Those things change often.

Comment 5

[Steffen Wilberg]

I meant: I guess that's you, Ben Goodger? Do you want me to change
[e-mail] to [e-mail]?
Ben is the guy I requested review from at the same time you know :)

Comment 6

[Steffen Wilberg]

Attachment: patch for the fonts, languages and connections dialogs

Comment 7

[Steffen Wilberg]

Comment on attachment 159347 [details] [diff] [review]
patch for the fonts, languages and connections dialogs

More Help buttons.

Comment 8

[Mike Connor [:mconnor]]

not a blocker, but nice-to-have.

Comment 9

[Jeff Walden (gone starting June 8)]

With the UI freeze at the beginning of October and the rejection of a few other
minor issues with the Help viewer UI, this won't make 1.0.  Retargeting...

Comment 10

[Steffen Wilberg]

Comment on attachment 159273 [details] [diff] [review]
patch for the main Options dialog

Bitrot is near! (Bug 274712 )

Comment 11

[Steffen Wilberg]

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

Comment 12

[Steffen Wilberg]

Targetting bugs which were targetted to Firefox1.1 before the move to
mozilla1.8beta2.

Comment 13

[Steffen Wilberg]

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

Comment 14

[Steffen Wilberg]

I'd be glad if anybody could give me a tip on how to add Help buttons in the new
preferences system.

Comment 15

[Steffen Wilberg]

Attachment: patch for the new world, v1

This patch adds Help buttons to all prefpanes of the prefwindow, as well as all
of its child prefwindows and dialogs: conncection settings, sanitize settings,
fonts, colors, change actions, languages.
It does not include the <window>s, because we need to decide whether and where
to add the button manually: allowed sites/exceptions, cookies, download
actions.
It also does not include the password manager and the set/change/remove master
password prefwindows, and the dialogs in the Advanced-Security-Certificates
section, because we have no useful documentation for that.

If the prefpane has tabs, the Help button is specific to that, so if you click
Help when the passwords tab of the privacy pane is displayed, the Help Viewer
displays the section on passwords.

The most difficult part was to get the Help button to appear. In dialogs, you
just specify buttons="accept,cancel,help". This doesn't work in prefwindows.
But since I wanted to make the Help button specific to the selected tab, I
needed a special implementation anyway.

The "makeHelpButton" method un-hides the Help button if the current prefpane
has a helpURI attribute. That method is called at the end of the "showPane"
method. The problem is that the prefpane may not be finished with loading,
especially when clicking the pane the first time after a restart of Firefox.
So I used a timeout to call makeHelpButton. It needs to be quite a long one, at
least on my Linux/GTK2 build, which is pretty slow in displaying the
Preferences dialog. Maybe this can be improved by using an onload handler.

The "dialogHelp" handler calls openHelp(helpTopic, helpURI) in contextHelp.js.
The helpTopic can be set as an attribute of prefpanes, or tabs.

Comment 16

[Jeff Walden (gone starting June 8)]

Moving this into the correct product/component, sorry for bugspam in advance...

Comment 17

[Asaf Romano (gone)]

Comment on attachment 182675 [details] [diff] [review]
patch for the new world, v1

>Index: mozilla/toolkit/content/widgets/preferences.xml
>===================================================================
>RCS file: /cvsroot/mozilla/toolkit/content/widgets/preferences.xml,v
>retrieving revision 1.14
>diff -u -p -8 -r1.14 preferences.xml
>--- mozilla/toolkit/content/widgets/preferences.xml	28 Apr 2005 21:49:46 -0000	1.14
>+++ mozilla/toolkit/content/widgets/preferences.xml	5 May 2005 14:13:11 -0000
>@@ -579,20 +579,39 @@
>               }
>             };
>             
>             var obs = new OverlayLoadObserver(aPaneElement);
>             document.loadOverlay(aPaneElement.src, obs);
>           }
>           else
>             this._selectPane(aPaneElement);
>+
>+#ifndef XP_MACOSX
>+          //Let the Help button stay hidden on Mac, like the other buttons
>+          //XXXsw make this an onload handler?
>+          setTimeout(this.makeHelpButton, 100, aPaneElement);
>+#endif
>         ]]>
>         </body>
>       </method>

err, we do want the help button on mac,  you need to fix preferences.xml to
only hide the relevant buttons (instead of their parwent)

Also, why is it setTimeoted? If it is a "dom isn't ready" case, "0" should be
fine.


>+      <handler event="dialoghelp">
>+        // 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];
>+        openHelp(pane.helpTopic, pane.getAttribute("helpURI"));
>+      </handler>

As you're using openHelp() in the binding, it has to provide this method
(that's instead of including an external script in every pane).

Comment 18

[Steffen Wilberg]

> err, we do want the help button on mac,  you need to fix preferences.xml to
> only hide the relevant buttons (instead of their parwent)
So there should be a Help button on Mac, but no Close (or Cancel/Accept)
buttons, right?

> Also, why is it setTimeouted? If it is a "dom isn't ready" case, "0" should be
> fine.
"0" doesn't work. The Help button would stay hidden the first time you display a
prefpane after a restart of Firefox. But I've found a better solution. There's
already a "OverlayLoadObserver", which is called in the showPane method if the
prefpane hasn't been loaded before. _selectPane is called either from that or
directly in showPane. So I can move the logic into _selectPane and don't need a
timeout.

Comment 19

[Steffen Wilberg]

Attachment: patch v1.1

This patch fixes the Help button on Mac, and gets rid of the setTimeout.

Comment 20

[Asaf Romano (gone)]

Well, ths still doesn't address the openHelp issue.

Comment 21

[Steffen Wilberg]

Attachment: include the contextHelp.js stuff into preferences.xml

I'm not completely convinced that we need to do this, so I made a separate
patch for this.

Comment 22

[Kevin Brosnan]

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

Comment 23

[Mike Connor [:mconnor]]

Comment on attachment 183267 [details] [diff] [review]
include the contextHelp.js stuff into preferences.xml

IMO, its better to have these as methods/properties of the binding, instead of
external dependencies

>+      // copied from contextHelp.js
>+      // Locate existing help window for this helpFileURI.
>+      <method name="locateHelpWindow">
>+        <paramter name="helpFileURI"/>
>+        <body>
>+          const windowManagerInterface = Components
>+              .classes['@mozilla.org/appshell/window-mediator;1'].getService()
>+              .QueryInterface(Components.interfaces.nsIWindowMediator);
>+          const iterator = windowManagerInterface.getEnumerator("mozilla:help");
>+          var topWindow = null;
>+          var aWindow;
>+
>+          // Loop through help windows looking for one with selected helpFileURI
>+          while (iterator.hasMoreElements()) {
>+              aWindow = iterator.getNext();
>+              if (aWindow.getHelpFileURI() == helpFileURI) {
>+                  topWindow = aWindow;
>+              }
>+          }
>+          return topWindow;
>+        </body>
>+      </method>

please fix the atrocious styling here to fit toolkit and not the
should-be-beaten-with-a-stick "style" that Help code used.  also,
s/windowManagerInterface/wm/ for compactness/readability/style.

>+      <property name="helpTopic">
>+        <getter>
>+          // first try the panel
>+          var helpTopic = this.getAttribute("helpTopic");
>+
>+          // are there tabs, and do they offer a helpTopic?
>+          var box = this.getElementsByTagName("tabbox");
>+          if (box[0])
>+            var tab = box[0].selectedTab;
>+          if (tab)
>+            if (tab.hasAttribute('helpTopic'))
>+              helpTopic = tab.getAttribute("helpTopic");
>+
>+          return helpTopic;
>+        </getter>
>+      </property>

you can refactor this in a much more straightforward/compact way, trying the
panel first only makes sense if it takes priority:

// if there's tabs, use the tab's helpTopic if set
var box = this.getElementsByTagName("tabbox");
if (box[0]) {
  var tab = box[0].selectedTab;
  if (tab && tab.hasAttribute("helpTopic");
    return tab.getAttribute("helpTopic");
}

return this.getAttribute("helpTopic");

r=me with comments addressed

I'm sure that the positioning of the solitary Help button in the main
prefwindow is going to be wrong, but that can be done as a followup by someone
more Mac-focused *cough*

Comment 24

[Steffen Wilberg]

Attachment: comments addressed

This patch addresses comment 23.

Comment 25

[Steffen Wilberg]

Attachment: comments addressed

Same as above, with some indentation fixes.

Comment 26

[Steffen Wilberg]

Checked in. Please file new bugs if you want more Help buttons, or don't like
its position on Mac :-)

Comment 27

[Steffen Wilberg]

Attachment: fix typo

Oops.

Comment 28

[Mike Connor [:mconnor]]

Comment on attachment 186638 [details] [diff] [review]
fix typo

man, how'd I miss that?

Comment 29

[Steffen Wilberg]

Comment on attachment 186638 [details] [diff] [review]
fix typo

Checked in.