260058 - Remove Help dependencies on Thunderbird and Firefox

Status: RESOLVED FIXED

(SeaMonkey :: Help Viewer, defect)

Description

[Steffen Wilberg]

All of the app-specific #ifdefs need to die for help to be embeddable in
XRE/xulrunner. They're only used to call Firefox Help on Firefox and Thunderbird
Help on Thunderbird.

The content pack url is currently hardcoded in contextHelp.js to
chrome://help/locale/firebirdhelp.rdf or
chrome://help/locale/thunderbirdhelp.rdf. I suggest to use a pref instead.

Comment 1

[Jeff Walden (gone starting June 8)]

(In reply to comment #0)
> The content pack url is currently hardcoded in contextHelp.js to
> chrome://help/locale/firebirdhelp.rdf or
> chrome://help/locale/thunderbirdhelp.rdf. I suggest to use a pref instead.

Having a pref for the name really seems to me to be useless bloat.  Just make
the name something generic and stick with it for everything.

I'm working primarily on some of the docs bugs now, but this is something I'd
really like to do as a learning experience about the back end of Help (and
probably about the build process as well).  If you really want to do it,
Steffen, do so, but if you can wait on this until I've tried or said I can't do
it, I'd appreciate it.

Comment 2

[Steffen Wilberg]

(In reply to comment #1)
> Just make the name something generic and stick with it for everything.
Sounds even better.
 
> I'm working primarily on some of the docs bugs now, but this is something I'd
> really like to do as a learning experience about the back end of Help (and
> probably about the build process as well).  If you really want to do it,
> Steffen, do so, but if you can wait on this until I've tried or said I can't
> do it, I'd appreciate it.
Sure, go ahead! I'm working on the context sensitive Help (bug 260054) right now.

I'm on #documentation btw.

Comment 3

[R.J. Keller]

I don't think you understand how help content packs work. firebirdhelp.rdf IS
the content pack. You can't have the content pack URL hard-coded into the
content pack :). By default, you're supposed to use the contextHelp.js functions
setURI(string)  to set the content pack loaded into help. The default is set to
firebirdhelp.rdf, but can be changed to another RDF file with appropriate settings.

You should not need to modify the content pack unless you're making a API change.

Comment 4

[Steffen Wilberg]

In contextHelp.js, there's this code:
#ifndef MOZ_THUNDERBIRD
const MOZILLA_CONTENT_PACK = "chrome://help/locale/firebirdhelp.rdf";
#else
const MOZILLA_CONTENT_PACK = "chrome://help/locale/thunderbirdhelp.rdf";
#endif

In helpMenuOverlay.xul, there's this:
#ifdef MOZ_THUNDERBIRD
  oncommand="openHelp('smtp-docs', 'chrome://help/locale/thunderbirdhelp.rdf');"/>
#else
  oncommand="openHelp('firefox-help');"/>
#endif

We want to get rid of these because the toolkit, in which these files live, is
supposed to become embeddable in xulrunner. Imagine a dozen more applications
using the Help Viewer. We don't want to add a dozen ifdefs.

So what do you suggest?
- Get rid of helpMenuOverlay.xul and move it to browser-menubar.inc and
browser-sets.inc (in Firefox). Use openHelp('firefox-help') there. Help would
become a mandatory extension.

- Use a app-specific pref which holds the content pack url
(chrome://help/locale/firebirdhelp.rdf or
chrome://help/locale/thunderbirdhelp.rdf). Query that pref in contextHelp.js.

- Make the content pack url generic, being the same for every app. Hardcode
that, but leave the option to change it via setHelpFileURI().

Comment 5

[R.J. Keller]

(In reply to comment #4)
> In contextHelp.js, there's this code:
> #ifndef MOZ_THUNDERBIRD
> const MOZILLA_CONTENT_PACK = "chrome://help/locale/firebirdhelp.rdf";
> #else
> const MOZILLA_CONTENT_PACK = "chrome://help/locale/thunderbirdhelp.rdf";
> #endif
> 
> In helpMenuOverlay.xul, there's this:
> #ifdef MOZ_THUNDERBIRD
>   oncommand="openHelp('smtp-docs', 'chrome://help/locale/thunderbirdhelp.rdf');"/>
> #else
>   oncommand="openHelp('firefox-help');"/>
> #endif
> 
> We want to get rid of these because the toolkit, in which these files live, is
> supposed to become embeddable in xulrunner. Imagine a dozen more applications
> using the Help Viewer. We don't want to add a dozen ifdefs.

I don't think you understand. MOZILLA_CONTENT_PACK is the default. You can
easily change that value. Really Thunderbird Help shouldn't be using the #ifdef
method, but instead should use setHelpFileURI().

> 
> So what do you suggest?
> - Get rid of helpMenuOverlay.xul and move it to browser-menubar.inc and
> browser-sets.inc (in Firefox). Use openHelp('firefox-help') there. Help would
> become a mandatory extension.

I never said that and wouldn't recommend it. You seem very confused about how
the API works. Read the Help Content Spec ->
http://www.mozilla.org/projects/help-viewer/content_packs.php

Also here's the code to change the content pack to something else ->
http://lxr.mozilla.org/mozilla/source/toolkit/components/help/content/contextHelp.js#76

Also note that you can take a content pack URI as a parameter to openHelp(),
just a little extra feature I added awhile ago ->
http://lxr.mozilla.org/mozilla/source/toolkit/components/help/content/contextHelp.js#50

The default content pack value should NEVER be modified. Why Thunderbird Help
modifies it via #ifdef is an error.

Comment 6

[R.J. Keller]

WORKSFORME. This is not a issue with the API and is a default function. Whatever
the default content pack is doesn't matter. People using the API shouldn't be
toggling the default value, just changing that value to their app at runtime or
using showHelp()'s URI parameter.

Comment 7

[Mike Connor [:mconnor]]

anything that's using #ifdefs in toolkit needs to die for xulrunner.  If the API
is specced that way, then the API spec needs to change.  End of story.

Using a pref is not bloat, we do the same for the browser chrome URL.  Its the
best solution for this, IMO.

Comment 8

[R.J. Keller]

(In reply to comment #7)
> anything that's using #ifdefs in toolkit needs to die for xulrunner.  If the API
> is specced that way, then the API spec needs to change.  End of story.
> 
> Using a pref is not bloat, we do the same for the browser chrome URL.  Its the
> best solution for this, IMO.

Did you read what I just said? Obviuosly not. having a pref would be USELESS.
Let Jeff decide. You are not a help peer so therefore you cannot make this
decision. If after READING my comments Jeff wants the bug reopened, then so be it.

Now don't think I disagree with removing the ifdef. Thunderbird help shouldn't
be modifying the default. Its sloppy coding and isn't what the API was designed
for. That should be fixed, but that isn't really what this bug is about. I
recommend filing a new one, but if a help peer disagrees, then reopen.

Comment 9

[R.J. Keller]

<Wolf> the code lives in toolkit, and appears to have a problem if you *need* to
modify the source to embed it, IMO
<rj_keller> Wolf: why would you need to modify it?
<rj_keller> Wolf: you're supposed to use setHelpFileURI() to set the content
pack, not change hte default.
<rj_keller> Actually, just the fact that we have a default content pack is a bug.

Comment 10

[Steffen Wilberg]

I don't agree we need a new bug. This bug is about removing app-specific ifdefs
from code in mozilla/toolkit/components/help, because toolkit should be somewhat
independant from Firefox and Thunderbird.

> <rj_keller> Actually, just the fact that we have a default content pack is a bug.
So let's remove it. But where else does contextHelp.js get the default pack
from? From a pref. Have a look at the pref browser.chromeURL. It stores the main
entry point for Firefox. It says: chrome://browser/content/

Why not introduce a pref help.defaultContentPack, set to
chrome://help/locale/firebirdhelp.rdf in firefox.js and to
chrome://help/locale/thunderbirdhelp.rdf in thunderbird.js? That would be the
default pack. Extensions wouldn't change it, but still use setHelpFileURI() to
provide their own content.

Comment 11

[R.J. Keller]

(In reply to comment #10)
> I don't agree we need a new bug. This bug is about removing app-specific ifdefs
> from code in mozilla/toolkit/components/help, because toolkit should be somewhat
> independant from Firefox and Thunderbird.
> 
> > <rj_keller> Actually, just the fact that we have a default content pack is a
bug.
> So let's remove it. But where else does contextHelp.js get the default pack
> from? From a pref. Have a look at the pref browser.chromeURL. It stores the main
> entry point for Firefox. It says: chrome://browser/content/
> 
> Why not introduce a pref help.defaultContentPack, set to
> chrome://help/locale/firebirdhelp.rdf in firefox.js and to
> chrome://help/locale/thunderbirdhelp.rdf in thunderbird.js? That would be the
> default pack. Extensions wouldn't change it, but still use setHelpFileURI() to
> provide their own content.

From the way I made the API, the default shouldn't really be used. In a perfect
world it should be null. You can do whatever you want, its out of my hands now.
 don't really care enough to say anything other than what is supposed to happen
in the way I designed the API. Just telling you that I didn't design the API to
work that way. Function calls are much easier for apps using multiple content
packs than prefs. If you want to rework the API, then at least update
content_help on the help viewer site so others know. But really I do not care. I
have bigger fish to fry than play around with Mozilla bugs.

Comment 12

[Steffen Wilberg]

I just discussed this with R.J.
The best solution is to get rid of the hard-coded default content pack, along
with all the ifdefs. We don't need a pref either. All we need to do is call
setHelpFileURI("chrome://help/locale/firebirdhelp.rdf"); on startup. That needs
to be added to delayedStartup() in browser.js. delayedStartup because it's not
visible and shouldn't block initializing the browser window. In order for that
to work, <script type="application/x-javascript"
src="chrome://help/content/contextHelp.js"/> needs to be added to
browser-scripts.inc.

Jeff, can you try if that works?

Comment 13

[Mike Connor [:mconnor]]

I have a serious problem with loading a global persistent variable from
delayedStartup(), there simply isn't a reason for this.

if you need the path, set it when you invoke help, not before.

Comment 14

[R.J. Keller]

(In reply to comment #13)
> I have a serious problem with loading a global persistent variable from
> delayedStartup(), there simply isn't a reason for this.
> 
> if you need the path, set it when you invoke help, not before.

thanks for your opinion. Obviously the bug reporter would understand the bug
better than anyone else and since hes a peer he gets the final say. IMO (not
that it means much), there simply isn't a reason for a pref. Why would you want
to constantly swap between prefs on a certain app as opposed to using a simple
function call? If you have trouble understanding, ping me on IRC.

Jeff, you need to get on IRC more often :). Most stuff like this gets discussed
there anyways. Got a tree available for some patch testing? I can make a patch
if you want but don't have a working tree (or working system for that matter
:)). Might be slow though. I don't have a whole lot of time.

Comment 15

[Mike Connor [:mconnor]]

RJ, with all due respect for the work you guys do on Help, you don't have
authority over browser.js.  

I think that having a persisted global var created at startup for each window is
a lot more bloaty than having a default chrome URL as a pref.  We don't hardcode
the base URL for the entire app, why would we do it for Help content?

Added bonus to using the pref, embeddors/corporates can create a sub/superset of
contents and push that to their users in place of the defaults.

Comment 16

[R.J. Keller]

(In reply to comment #15)
> RJ, with all due respect for the work you guys do on Help, you don't have
> authority over browser.js.

and neither do you.

Comment 17

[Ben Goodger (use ben at mozilla dot org for email)]

It seems to me that the simplest solution might be to use the same name for the
content pack RDF file for all apps, rather than put the app name into the filename. 

Also, it looks like you're packaging your locale files into help.jar, not
en-US.jar, which makes it more difficult to localize, and difficult to update
the language pack a browser is running with or the help engine independently of
each other - ideally help.jar would contain just the UI/engine and thus the jar
file would be the same for *all* languages. 

Comment 18

[Steffen Wilberg]

I agree with Ben. That's the third suggestion I made in comment 4. Let's go for
that.

I filed bug 260417 about packaging Help content in en-us.jar.

Comment 19

[Jeff Walden (gone starting June 8)]

If this isn't done, Help won't coexist well in Thunderbird and Firefox together.
 Thunderbird's coming from the branch, so this needs to be fixed in aviary.  As
such, I'm retargeting it for 1.0.  As long as docs issues get fixed (they're
coming together, and I'm pretty much doing what I can there already), there's
plenty of time to fix this before 1.0.

See end of comment for a few questions I need answered...

(In reply to comment #17)
> It seems to me that the simplest solution might be to use the same name for
> the content pack RDF file for all apps, rather than put the app name into the
> filename.

i.e., chrome://help/locale/defaultContentPack.rdf?  That's what I'm doing in the
work-in-progress patch I'm making (via jar.mn for now, but perhaps renaming it
on the CVS server side is a better idea).  The patch will also make
"openHelp();" a valid call by setting a default topic (marked by
ID="default-topic" in the table of contents RDF file).  The way I've implemented
it, the code shouldn't interfere with the old API.

Anyways, an overview of what's happening (partly as an FYI, partly so I can
remember what I've done when I get back to working on the patch later):

Issues to fix:
-contents.rdf #ifdefs
  -ifdefs used to get overlays working
    -can't count on a specific name for files to overlay
    -SOLUTION: move contents.rdf into app-specific Help
-helpMenuOverlay.xul
  -ifdef used to overlay F1/Help key access to Help
    -problematic: keysets that are overlaid have different @id values
    -SOLUTION: file a bug for Thunderbird, have mscott change the name (eurgh,
     probably not super-trivial)?  create default keyset for all next-gen apps?
     ideas, anyone?
  -ifdef used to sort out different openHelp() arguments
    -this is also kinda bad
    -SOLUTION: set up openHelp() to accept 0 arguments and load a default topic
     (as outlined above)
      -default topic requires changing ID="default-topic" in help topic RDF
      -make help RDF default be what's used by app Help - not difficult to do in
       a backwards-compatible way, I think
  -openHelp used twice, might consider a <command/> - but orthogonal to this bug
  -ifdef used to display "For IE Users" in Fx
    -bad idea, w/o ifdef it'd show in Thunderbird
    -SOLUTION: add overlay into browser/components/help for the extra entry
      -would work well with the contents.rdf move mentioned above
-toolkit/components/help/Makefile.in
  -ifdef used to eventually let apps w/o browser toolbar (Tb) coexist with
   those that do (Fx)
    -can't just remove because images needed by Help
    -SOLUTION: give Help a default skin, but use + in jar.mn to override it
     with an app-specific skin in browser/components/help (build process allows
     this)
      -don't quite know yet how that would work with the toolbar image - how to
       override *its existence* and replace with nothing?
        -actually a current problem - e.g., chrome://help/skin/Toolbar.png is
         in builds but is unused - *bloat/cruft*
-contextHelp.js
  -default topics for Fx/Tb are hardcoded into file
  -SOLUTION: set default topic name, make Fx/Tb obey that name
    -also changes to jar.mn in browser/components/help, change in ToC RDF
    -extend Help API so openHelp() is callable w/o args to get default topic
    -a duplicate of some helpMenuOverlay.xul changes mentioned above
-toolkit/themes/qute/help/help.css
  -(would be) used by TB Help (?) - could check but don't care to right now

I think that's it, and I believe I've covered the app ifdefs in toolkit Help. 
Anyways, the questions that need answers:

-Can I diff files in a new directory without creating the directory on cvs.m.o?
-Should I file a bug for mscott to change the name of the main keyset in TB?  Or
 would another way be better?

Answers appreciated, preliminary patch sometime soon, likely after I take care
of a few things that require my time this week...

Comment 20

[Steffen Wilberg]

> -helpMenuOverlay.xul
>   -ifdef used to overlay F1/Help key access to Help
>     -problematic: keysets that are overlaid have different @id values
>     -SOLUTION: file a bug for Thunderbird, have mscott change the name (eurgh,
>      probably not super-trivial)?  create default keyset for all next-gen apps?
Either that, or let each app handle its Help menu itself. That would solve the
"for IE users" menuitem problem as well. Move helpMenuOverlay.xul into browser
and mail.

> -toolkit/components/help/Makefile.in
There's something pretty wrong in that makefile:
# Use Qute on non-Mac non-Phoenix apps
Oops, there's no qute folder in toolkit/components/help/skins anymore. Skin now
lives over there:
http://lxr.mozilla.org/aviarybranch/source/toolkit/themes/pinstripe/help/
http://lxr.mozilla.org/aviarybranch/source/toolkit/themes/qute/help/
http://lxr.mozilla.org/aviarybranch/source/toolkit/themes/winstripe/help/

The ifdef between the themes is done here:
http://lxr.mozilla.org/aviarybranch/source/toolkit/themes/Makefile.in
That's somebody else's problem. But we need to remove the bogus ifdef in
toolkit/components/help/Makefile.in and remove the skins subdirectory.

>   -ifdef used to eventually let apps w/o browser toolbar (Tb) coexist with
>    those that do (Fx)
We're indeed referencing chrome://browser/skin/Toolbar.png in
toolkit/themes/pinstripe/help/help.css
toolkit/themes/qute/help/help.css (#ifndef MOZ_PHOENIX)
toolkit/themes/winstripe/help/help.css

>     -can't just remove because images needed by Help
Indeed, for the back/forwards/home/print buttons.

>     -SOLUTION: give Help a default skin, but use + in jar.mn to override it
>      with an app-specific skin in browser/components/help (build process
>      allows this)
I think using browser buttons is a hack. Ask Kevin to update these files:
http://lxr.mozilla.org/aviarybranch/source/toolkit/themes/winstripe/help/Toolbar.png
http://lxr.mozilla.org/aviarybranch/source/toolkit/themes/pinstripe/help/Toolbar.png
and back out bug 252974.
The qute version has another problem: the zoom and toggle sidebar buttons are in
a separate file zoomImg.png.

>         -actually a current problem - e.g., chrome://help/skin/Toolbar.png is
>          in builds but is unused - *bloat/cruft*
Sorry, that's not true. We're using it for the help-sidebar-button,
help-find-button, help-zoom-small-button, help-zoom-large-buttonm
help-toolbar-customization. See help.xul: it has a class="toolbarbutton-1"

> -toolkit/themes/qute/help/help.css
>   -(would be) used by TB Help (?) - could check but don't care to right now
right.

> I think that's it, and I believe I've covered the app ifdefs in toolkit Help. 
> Anyways, the questions that need answers:
> 
> -Can I diff files in a new directory without creating the directory on cvs.m.o?
No. But that's not a problem. Just cvs add the directory and the files in it.
It's not visible to others until you cvs commit that. Use cvs diff -uN (N for
new files).

Thanks a lot for taking this. Writing this comment was time-consuming enough. :)

Comment 21

[Mike Connor [:mconnor]]

I think I'm missing something here.  Last I heard, Thunderbird 1.0 wasn't going
to ship with a Help Viewer, so based on that I see zero justification for
taking/making these changes on the branch.  This isn't fixing a visible bug that
will affect Firefox or Thunderbird, so why are we going to make changes?

If you want to fix it sooner, great, but it should stay on trunk unless there's
a very compelling reason to make these changes on branch.  Nothing I've read
gives me that so far, but I'm open to reasoning as to why we need to take a
calculated risk.  (Keep in mind that EVERY patch going onto aviary has to meet
an appropriate risk/reward factor.)

Comment 22

[Jeff Walden [:Waldo]]

(In reply to comment #21)
> I think I'm missing something here.  Last I heard, Thunderbird 1.0 wasn't
> going to ship with a Help Viewer, so based on that I see zero justification
> for taking/making these changes on the branch.

Actually, I think *I'm* the one missing something here.

Some sort of built-in Help is an essential part of *any* software (even moreso
in the case of something as complex as an email client -- does Gmail with its
interface not have Help?).  I *really* wish I'd gotten in the loop earlier,
because this is the first mention I've heard of this decision (if indeed it's a
set-in-stone, already-made decision that can't change).

That said, I don't see a reason that I couldn't sync Help content on the branch
with Help content on the trunk, create a patch for trunk, let the changes bake
for a while, and then backport them to aviary (either before or after Firefox
1.0, probably wouldn't matter much unless you absolutely refuse changes that
might somehow conceivably introduce instability).  This way, Help in Thunderbird
1.0 is still a possibility.

I guess for now I'll do the work on trunk, but not having Help in Thunderbird
for a "1.0" release strikes me as a very, very bad decision.

Comment 23

[Mike Connor [:mconnor]]

Is there actually complete and ready documentation for Thunderbird Help?  Its
really really late in the game to add this, especially since we're at the l10n
freeze, and there's currently nothing.  If freezing now is needed for localizing
Firefox itself, creating complete Help content for Thunderbird is probably out
of the question for other localizations.  If its ready _now_ we could possibly
try to make a case to the localizers, but we'd probably have to let it slide. 
If its a week from now, there is basically zero chance.  People are already
complaining about localizing 900 strings for the Tbird UI, forcing Help content
on them is just not going to happen.

As for the "could possibly introduce stability issues" we're only taking patches
on stuff that's user-visible in some way.  Underlying architecture issues and
dependencies aren't part of that category.  Even if Tbird _was_ shipping Help, I
don't think we'd have enough of a reason to land this on branch, since in theory
there is no embedding requirement on the 1.0 branch.  xulrunner is trunk-only at
this point.

Comment 24

[Jeff Walden (gone starting June 8)]

(In reply to comment #23)
> Is there actually complete and ready documentation for Thunderbird Help?  Its
> really really late in the game to add this, especially since we're at the l10n
> freeze, and there's currently nothing.

Oh, so it's just a pragmatic decision?  I suppose I can't argue with you there,
although it's really sad how little attention Thunderbird gets compared to
Firefox.  I guess that's what happens when the Foundation can't put as much
effort into it as it can in Firefox.

Comment 25

[Steffen Wilberg]

I haven't heard of a decision regarding Help in Thunderbird 1.0.
But the current state of Thunderbird Help content is very poor. There's nothing
in the tree, and http://www.mozilla.org/projects/help-viewer/installation.php
only provides a "0.1 Beta" version as an extension, which contains almost no
content and doesn't even work. I don't know if there's something newer in mozdev
cvs.

The Thunderbird 1.0 is scheduled together with Firefox 1.0, which means end of
October right now. The l10n isn't necessarily an issue, since localizers don't
need to include Help. They don't even have to include Firefox Help. Maybe we
could copy some of Seamonkey Help, but it's still a terrible amount of work, and
I doubt anyone is likely to contribute some 50-100 hours in the next few weeks.

However, this bug is not necessary for Thunderbird 1.0 Help. The app-specific
#ifdefs are in place to make Thunderbird Help possible. We don't need to remove
those for Thunderbird Help (only the start page has to be changed). We want to
remove them to make toolkit embeddable, i.e. not dependent on Firefox or
Thunderbird.

Comment 26

[R.J. Keller]

I definetely agree with mconnor. If you want to scrap the ifdefs, then burn the
Thunderbird ones. They're never used anyways. Just there for the future. MozDev
isn't any more recent than whats on the Help Viewer page. I know because I've
been looking at it.

Not sure what path you're going, but if you're going against ifdefs, then
pretend like Thunderbird Help doesn't exist, because it basically doesn't.
Hopefully someone will get it for 2.0 :).

The real hard part is the platform classes. Those are really useful for the help
documentation and used often, making them difficult to remove. Those ifdefs will
also likely break embeddable Help. I haven't been tracking this bug so it
might've been mentioned but just something to bring up if it hasn't yet.

Comment 27

[Jeff Walden (gone starting June 8)]

(In reply to comment #26)
> The real hard part is the platform classes. Those are really useful for the help
> documentation and used often, making them difficult to remove. Those ifdefs will
> also likely break embeddable Help.

Not at all, actually.  The bug's only for "app-specific" ifdefs: #ifdef
MOZ_PHOENIX, #ifndef MOZ_THUNDERBIRD, etc.  #ifdef XP_WIN is perfectly okay (and
would really, really break the toolkit if they couldn't be there).

Comment 28

[Jeff Walden (gone starting June 8)]

Attachment: Move Help menu, F1 overlay into browser

I have been doing some work on this in preparation for making the Help Viewer
work in Thunderbird, but right now my laptop seems to be overheating during
times of intense activity -- such as at the very end of the build process
(usually during TestGtkNotebookEmbed or whatever the real name is). 
Consequently, this patch is untested in the full formal build.

However, because the build completes 99.9% of the way, I have been able to
successfully do partial builds to create Makefile's through the tree.  Then, I
can create an updated help.jar by using make in
(browser|toolkit)/components/help and toolkit/themes/winstripe/help.  I've
tested the patched help.jar with the 20041216 nightly on FC3, and nothing seems
amiss.

This patch gets rid of all the app-specific ifdefs in Help code, but it still
leaves some browser-dependent Help skin in toolkit.  That code will be fixed in
a later patch after this patch gets reviewed and applied.

Comment 29

[Steffen Wilberg]

The patch collides with the one in bug 267227, which proposes to get rid of
helpMenuOverlay.xul altogether. We might do this as a first step however, since
it's straight-forward, whereas bug 267227 is more complicated.

Comment 30

[Jeff Walden [:Waldo]]

Comment on attachment 168955 [details] [diff] [review]
Move Help menu, F1 overlay into browser

Obsoleted by bug 267227, awaiting that fix to continue with fixes here...

Comment 31

[Jeff Walden (gone starting June 8)]

Putting a useful URL here so I don't have to recreate it if I need it again...

http://snurl.com/find_bar_help_xul

Comment 32

[Jeff Walden (gone starting June 8)]

Attachment: Remove #ifdef for default content pack name

This patch removes the platform-specific #ifdef that's used to set the default
Help content pack RDF file.  I don't really know why we even had the #ifdef in
the first place as it serves no real purpose, but with this patch checked in
we'll have one less of them.

In the long term it probably makes sense to remove the default content pack
altogether.  If the content pack provided in the call to openHelp() or set
earlier by setHelpFileURI() doesn't exist, we would then simply return an
error.	However, that change can wait until later.

Comment 33

[Steffen Wilberg]

Comment on attachment 178540 [details] [diff] [review]
Remove #ifdef for default content pack name

Restoring mconnor's review+

Comment 34

[Steffen Wilberg]

Comment on attachment 178540 [details] [diff] [review]
Remove #ifdef for default content pack name

Bitrotted.

Comment 35

[Worcester12345]

?-ing since a patch exists. Bitrot?

Comment 36

[Jeff Walden (gone starting June 8)]

Fixed by bug 268776.  Everyone who was holding off from using the toolkit Help
viewer because it wasn't app-independent should now be able to use it without
unorthodox hacks (assuming, of course, that they ship a more or less proper
representation of toolkit - those who don't really ship toolkit
*coughThunderbird* need not apply).