in some dialogs in Thunderbird --namely the seamonkey-based ones-- clicking the Help (or ?) buttons doesn't bring up the Help window (currently doesn't do anything other than throwing errors in the js console). I'll list the dialogs affected by this. Account Settings dialog (Help button) Error: openHelp is not defined Source File: chrome://messenger/content/am-help.js Line: 78 LDAP Directory Servers (Help) Error: openHelp is not defined Source File: chrome://messenger/content/addressbook/pref-directory.js Line: 518 Directory Server Properties (Help) Error: openHelp is not defined Source File: chrome://messenger/content/addressbook/pref-directory-add.js Line: 269 Certificate Manager (Help) Error: openHelp is not defined Source File: chrome://pippki/content/certManager.js Line: 112 Device Manager (Help) Error: openHelp is not defined Source File: chrome://global/content/bindings/dialog.xml Line: 291 will add more in a bit...

Message Filters (Help) Error: openHelp is not defined Filter Rules (Help) Error: openHelp is not defined Source File: chrome://messenger/content/FilterEditor.js Line: 729 Customize Message Views (Help) Error: openHelp is not defined Source File: chrome://global/content/bindings/dialog.xml Line: 291 Message View Setup Error: openHelp is not defined Source File: chrome://messenger/content/mailViewSetup.js Line: 128

Message Security dialog (from mail compose) Error: openHelp is not defined Source File: chrome://messenger-smime/content/msgCompSecurityInfo.js Line: 282 Password Manager (from Options > Advanced) Error: openHelp is not defined Source File: chrome://communicator/content/wallet/SignonViewer.js Line: 712 Change Master Password (from Options > Advanced) Error: openHelp is not defined

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

Message security dialog (from either menus in main window or in message reading window) Error: openHelp is not defined Source File: chrome://messenger-smime/content/msgReadSecurityInfo.js Line: 263 Filter rules dialog (NOT the filter editor dialog) doesn't show a line/file in bugzilla Sorry for posting the dupe. (I searched on the S/Mime extension, which I initially thought was to blame). What should be done about these? Does another file from seamonkey need to be included into TB? Are these help boxes going to disappear?

this simple fix addresses all of the dialogs Sarah listed above with the following EXCEPTIONS: 1) Message Security dialog (from mail compose) 2) Master Password --> Change Password dialog This is because these old security style dialogs are hard coding the help/ok/cancel buttons instead of using the generic dialog overlay to get them. I doubt we'll bother to fix those two dialogs. This gets us almost all of the way there with little cost.

fixed branch and trunk

everything looks fine, with the exception of the Change Master Password dialog, as noted in comment 5.

(In reply to comment #5) > this simple fix addresses all of the dialogs Sarah listed above with the > following EXCEPTIONS: > > 1) Message Security dialog (from mail compose) Bug 268540.

Can I propose reopening this on the basis that extension developers may want to include a help button on dialogs within TB but this patch has made it necessary to go the old manual route. I've done a quick search on mailnews on lxr and find 11 instances of dialogs using help in their buttons attribute. http://lxr.mozilla.org/mailnews/search?string=%2Chelp If I was to submit a patch to #ifdef the help button out of those files and reinstate the possibility of having a standard help button on dialogs, would this be considered? Are there other cases where a dialog will show a help button that's not included in that list? The help button can be handled in TB via the ondialoghelp attribute, and since there isn't a standard help facility I don't see this adding any further confusion to users, but making inclusion of help links in extensions slightly easier for authors.

Actually, I think this is simpler. If the XUL defines an ondialoghelp attribute, then show the help button. This should work since it's not going to expect the openHelp function to be available as the handler is overridden.

Comment on attachment 242889 [details] [diff] [review] Show the help button is ondialoghelp is defined You might want to ask a toolkit peer for review, but even then I'm pretty sure they are not going to want a thunderbird ifdef in dialog.xml. Hmm...

(In reply to comment #11) > (From update of attachment 242889 [details] [diff] [review] [edit]) > You might want to ask a toolkit peer for review, but even then I'm pretty > sure they are not going to want a thunderbird ifdef in dialog.xml. The ifdef is already there...

Actually, the ifdef was mscotts' creation to start with, but nevermind, we have another plan. Neil described to me an overlay, like the security one used elsewhere on SM, to add the help buttons manually. I've worked up an overlay to suit the remaining incidents of ",help" in mailnews, I just need to put a patch together to apply the overlay and remove the ",help" from the dialog/@buttons The overlay, I believe, will need applying to http://lxr.mozilla.org/mailnews/search?string=%2Chelp and that's what it provides for. http://lxr.mozilla.org/mailnews/source/mailnews/addrbook/prefs/resources/content/pref-editdirectories.xul doesn't have an @id, so there's a patch to add one. mark@standard8 mentioned that if TB is run on top of XUL runner, since MOZ_THUNDERBIRD is not defined there.

Add the missing id attribute to the one dialog missing it

Once part-1 is out the way, this should seamlessly slot in. Please note these diffs are blind, I have no SM build setup here.

Once part-1 and part-2 are in, this should remove the need for the hack in dialog.xml

Comment on attachment 242917 [details] [diff] [review] overlay help button and ondialoghelp to mailnews dialogs I forgot to point out that extensions/help/resources/jar.mn needs a change. r=me given that the new file is added to jar.mn and the typo below is fixed. >+ <RDF:li resource="chrome://messenger-smime/contentmsgReadSecurityInfo.xul"/> Typo: missing / after content >+<overlay id="helpMessengerOverlay" >+ xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"> You could add the contextMenu.js script here and remove it from the dialogs.

Comment on attachment 242919 [details] [diff] [review] remove help and ondialoghelp attributes from affected files > <script type="application/x-javascript" src="chrome://help/content/contextHelp.js"/> This is one of those lines that could be moved to the overlay. >+ ondialogcancel="return subscribeCancel();"> Trailing whitespace is frowned upon. >- ondialogaccept="return window.close();" Actually this is the default action for accept so it could just be removed.

Touched up after Neils' comments

Comment on attachment 242959 [details] [diff] [review] remove typo, add jar.mn, add contextHelp.js r=me with these nits fixed: > content/help/helpContextOverlay.xul (content/helpContextOverlay.xul) > content/help/helpSecurityOverlay.xul (content/helpSecurityOverlay.xul) > * content/help/platformClasses.css (content/platformClasses.css) >+ content/help/helpMessengerOverlay.xul (content/helpMessengerOverlay.xul) Nit: we normally keep these in alphabetical order >Index: resources/content/helpMessengerOverlay.xul >=================================================================== >RCS file: resources/content/helpMessengerOverlay.xul >diff -N resources/content/helpMessengerOverlay.xul >--- /dev/null 1 Jan 1970 00:00:00 -0000 >+++ resources/content/helpMessengerOverlay.xul 1 Jan 1970 00:00:00 -0000 Nit: this file has been differenced from mozilla/extensions/help but the other two files were differenced from mozilla/extensions/help/resources >+ Nit: (trailing) spaces on line

Comment on attachment 242959 [details] [diff] [review] remove typo, add jar.mn, add contextHelp.js >+ <script type="application/x-javascript" Not having much luck with those trailing spaces are you ;-)

I tried all the ways I can think of to check there's no trailing whitespace. You will, of cource, find one :D

Comment on attachment 243082 [details] [diff] [review] overlay help button and ondialoghelp to mailnews dialogs (v4) FYI, the * before an entry in a jar.mn file causes us to run the XUL pre-processor on the file before it gets put into the JAR file. So any XUL or JS file that uses an ifdef like #ifdef MOZ_THUNDERBIRD #endif needs an * entry in the jar.mn file for the xul pre-processor to be invoked. This patch looks good to me. Thanks for working on it. Please get a module owner review for these help changes before checking them in.

last piece of the puzzle to remove the earlier patch to hide help buttons in TB

Comment on attachment 243250 [details] [diff] [review] remove hack from dialog.xml trying to find the correct sr for /toolkit/content/widgets/dialog

Comment on attachment 243250 [details] [diff] [review] remove hack from dialog.xml r=me, assuming thunderbird no longer needs this hackaround (I hope not!)

Assuming the @id is added https://bugzilla.mozilla.org/attachment.cgi?id=242916 the overlay is added https://bugzilla.mozilla.org/attachment.cgi?id=243082 we remove the help and ondialoghelp stuff https://bugzilla.mozilla.org/attachment.cgi?id=242960 and we finally remove the hack https://bugzilla.mozilla.org/attachment.cgi?id=243250 all should be well in fairyland I think I've got all the r+ and sr+ needed. You guys with commmit privs need to do the rest. I don't want to keep pumping r=? and sr=? out so someone needs to pick this up from here, I can't commit.

OK, I think it's all in, let's hope it all works :-)