Closed Bug 260054 Opened 20 years ago Closed 19 years ago

add context sensitive Help

Categories

(Firefox :: Settings UI, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox1.5

People

(Reporter: steffen.wilberg, Assigned: steffen.wilberg)

References

Details

Attachments

(2 files, 6 obsolete files)

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.
Attached patch patch for the main Options dialog (obsolete) (deleted) — Splinter Review
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!?
Target Milestone: --- → Firefox1.0
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 on attachment 159273 [details] [diff] [review] patch for the main Options dialog found in nsPrefWindows.js: * CHANGES MUST BE REVIEWED BY ben@netscape.com!! That's you I guess!? Do you want me to change your email address as well?
Attachment #159273 - Flags: review?(bugs)
(In reply to comment #3) > (From update of attachment 159273 [details] [diff] [review]) > found in nsPrefWindows.js: > * CHANGES MUST BE REVIEWED BY ben@netscape.com!! > > 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.
I meant: I guess that's you, Ben Goodger? Do you want me to change ben@netscape.com to bugs@bengoodger.com? Ben is the guy I requested review from at the same time you know :)
Comment on attachment 159347 [details] [diff] [review] patch for the fonts, languages and connections dialogs More Help buttons.
Attachment #159347 - Flags: review?(bugs)
Flags: blocking-aviary1.0?
Whiteboard: [have patch]
not a blocker, but nice-to-have.
Flags: blocking-aviary1.0? → blocking-aviary1.0-
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...
Target Milestone: Firefox1.0 → After Firefox 1.0
Status: NEW → ASSIGNED
Whiteboard: [have patch]
Target Milestone: Future → Firefox1.1
Comment on attachment 159273 [details] [diff] [review] patch for the main Options dialog Bitrot is near! (Bug 274712 )
Attachment #159273 - Attachment is obsolete: true
Attachment #159273 - Flags: review?(bugs)
Attachment #159347 - Attachment is obsolete: true
Attachment #159347 - Flags: review?(bugs)
*** Bug 283651 has been marked as a duplicate of this bug. ***
Flags: blocking-aviary1.0-
Product: Firefox → Toolkit
Target Milestone: Firefox1.1 → ---
Targetting bugs which were targetted to Firefox1.1 before the move to mozilla1.8beta2.
Target Milestone: --- → mozilla1.8beta2
*** Bug 287819 has been marked as a duplicate of this bug. ***
I'd be glad if anybody could give me a tip on how to add Help buttons in the new preferences system.
Keywords: helpwanted
Attached patch patch for the new world, v1 (obsolete) (deleted) — Splinter Review
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.
Attachment #182675 - Flags: first-review?(mconnor)
Moving this into the correct product/component, sorry for bugspam in advance...
Component: Help Viewer → Preferences
Flags: first-review?(mconnor)
Product: Toolkit → Firefox
QA Contact: help → preferences
Target Milestone: mozilla1.8beta2 → Firefox1.1
Version: unspecified → Trunk
Attachment #182675 - Flags: review?(mconnor)
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).
Attachment #182675 - Flags: review?(mconnor) → review-
> 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.
Attached patch patch v1.1 (obsolete) (deleted) — Splinter Review
This patch fixes the Help button on Mac, and gets rid of the setTimeout.
Attachment #182675 - Attachment is obsolete: true
Well, ths still doesn't address the openHelp issue.
I'm not completely convinced that we need to do this, so I made a separate patch for this.
Attachment #183265 - Attachment is obsolete: true
Attachment #183267 - Flags: review?(mconnor)
*** Bug 268299 has been marked as a duplicate of this bug. ***
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*
Attachment #183267 - Flags: review?(mconnor) → review+
Attached patch comments addressed (obsolete) (deleted) — Splinter Review
This patch addresses comment 23.
Attachment #183267 - Attachment is obsolete: true
Attachment #186240 - Flags: review+
Attachment #186240 - Flags: approval-aviary1.1a2?
Attached patch comments addressed (deleted) — Splinter Review
Same as above, with some indentation fixes.
Attachment #186240 - Attachment is obsolete: true
Attachment #186242 - Flags: review+
Attachment #186242 - Flags: approval-aviary1.1a2?
Attachment #186240 - Flags: review+
Attachment #186240 - Flags: approval-aviary1.1a2?
Attachment #186242 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Blocks: 295904
Whiteboard: [checkin needed]
Checked in. Please file new bugs if you want more Help buttons, or don't like its position on Mac :-)
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Keywords: helpwanted
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Attached patch fix typo (deleted) — Splinter Review
Oops.
Attachment #186638 - Flags: review?(mconnor)
Attachment #186638 - Flags: approval-aviary1.1a2?
Comment on attachment 186638 [details] [diff] [review] fix typo man, how'd I miss that?
Attachment #186638 - Flags: review?(mconnor) → review+
Attachment #186638 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Comment on attachment 186638 [details] [diff] [review] fix typo Checked in.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: