344243 - move suiterunner help over to toolkit help viewer

Status: RESOLVED FIXED

(SeaMonkey :: Help Documentation, defect)

Description

[Robert Kaiser]

For suiterunner, we should use the toolkit help viewer for the help window, and we should move over help content to suite/locales/

I'm working on a patch to achieve that, I should have a preliminary version very soon.

Comment 1

[Robert Kaiser]

Attachment: preliminary patch to use toolkit help viewer for suiterunner

Here's the way I want to go for this.
The patch may not apply cleanly to trunk as I've hacked the diff itself heavily to reflect a patch aginst what the tree will be looking like after a few pending cvs copies and checkins (my communcator chrome moves).
I'll attach a cvs copy file as well in a minute, basically all the help content needs to be copied over into suite/

After applying this work, still some cleanup needs to be done to get style working correctly and remove redundancy of the "using help" content.

Comment 2

[Robert Kaiser]

Attachment: cvs copy file for moving seamonkey help content over into suite/

These are the cvs copies we need along with the poatch in this bug to get SeaMonkey help content into suite/locales/

Comment 3

[[e-mail]]

xulrunner is a pain the backside. Why can't we have the chrome we want?

Can we just ignore toolkit and install all our help under communicator?

Comment 4

[Robert Kaiser]

(In reply to comment #3)
> Can we just ignore toolkit and install all our help under communicator?

We probably could, but what's wrong with using their help viewer? I think we should try to share as much as possible with them, so that the amount of code we need to care about gets to be the stuff that really makes a difference.

Additionally, splitting between chrome://help and chrome://suitehelp seems to me like a good idea because it splits help viewer from help content, which are two different things actually.

Comment 5

[[e-mail]]

(In reply to comment #4)
>We probably could, but what's wrong with using their help viewer?
Because of all the other changes we need to get it to work?

Comment 6

[Robert Kaiser]

(In reply to comment #5)
> (In reply to comment #4)
> >We probably could, but what's wrong with using their help viewer?
> Because of all the other changes we need to get it to work?

Let's see... The configure.in would be needed in some way anyways, as we're (correctly) removing help from extensions/ dir. The jar.nm changes are needed in any case. The toolkit/components changes are something we want to do some time anyways, as we want/need to build toolkit without any ifdefs in the future. Adding the one pref is very trivial, and utilityOverlay stuff is only moving some part from a now uneeded overlay (as it's no extension any more) directly into that file. The suitehelp.rdf changes are only cosmetical anyways.

So I don't see "all the other changes" that are that strange to do... It wasn't as easy as I would have liked to find out what to do, but it's all stuff we need to do in any case or is trivial, IMO. The biggest thing we could discuss is if we'd want to keep that helpOverlay, but I think it's useless to keep it when we don't treat help as optional any more.

Comment 7

[[e-mail]]

Comment on attachment 228818 [details]
cvs copy file for moving seamonkey help content over into suite/

Can't review this until I can apply attachment 228817 [details] [diff] [review].

Comment 8

[Robert Kaiser]

Attachment: patch, v1: to use toolkit help viewer for suiterunner

This patch is against clean trunk and should work... I'm still in the process of building with it, but it should work.

Comment 9

[Robert Kaiser]

Comment on attachment 228818 [details]
cvs copy file for moving seamonkey help content over into suite/

requesting reviews again now that there's a patch that can be applied to trunk

Comment 10

[Robert Kaiser]

Attachment: patch, v1.1: to use toolkit help viewer for suiterunner

Sorry, I had missed the jar.mn change when porting this over to normal trunk. As we're moving preprocessed code from helpOverlay to utilityOverlay, we now have to preprocess the utilityOverlay

Comment 11

[[e-mail]]

Comment on attachment 229072 [details] [diff] [review]
patch, v1.1: to use toolkit help viewer for suiterunner

>+  if test "$MOZ_XUL_APP"; then
>+    MOZ_EXTENSIONS_DEFAULT=" wallet xml-rpc p3p venkman inspector irc typeaheadfind spellcheck gnomevfs sroaming reporter"
>+  else
>+    MOZ_EXTENSIONS_DEFAULT=" wallet xml-rpc help p3p venkman inspector irc typeaheadfind spellcheck gnomevfs sroaming reporter"
>+  fi
This needs build review.

>+% content suitehelp %content/suitehelp/ xpcnativewrappers=yes
I am really not keen on the use of suitehelp as a package.
Can we not put the docs under chrome://communicator/locale/help/ ?

> 	typeaheadfind \
But you didn't disable extensions/typeaheadfind. Do the two not conflict? Or are you cheating by building toolkit chrome but overriding the component?

>+                oncommand="openHelp('welcome', 'chrome://suitehelp/locale/help.rdf');"/>
How are we going to handle all the existing openHelp calls?

Comment 12

[Robert Kaiser]

(In reply to comment #11)
> (From update of attachment 229072 [details] [diff] [review] [edit])
> >+  if test "$MOZ_XUL_APP"; then
> >+    MOZ_EXTENSIONS_DEFAULT=" wallet xml-rpc p3p venkman inspector irc typeaheadfind spellcheck gnomevfs sroaming reporter"
> >+  else
> >+    MOZ_EXTENSIONS_DEFAULT=" wallet xml-rpc help p3p venkman inspector irc typeaheadfind spellcheck gnomevfs sroaming reporter"
> >+  fi
> This needs build review.

Sure, will try to get that...

> >+% content suitehelp %content/suitehelp/ xpcnativewrappers=yes
> I am really not keen on the use of suitehelp as a package.
> Can we not put the docs under chrome://communicator/locale/help/ ?

Sure we can, I just though we might want to have it in a separate chrome package, but I'm OK with sticking it into communicator as well.

> > 	typeaheadfind \
> But you didn't disable extensions/typeaheadfind. Do the two not conflict? Or
> are you cheating by building toolkit chrome but overriding the component?

Apparently they either don't conflict or override each other in a way that everything works. We really should disable extensions/typeaheadfind, I guess, but I didn't dare to risk this in the first patch, as we still can clean it up afterwards.

> >+                oncommand="openHelp('welcome', 'chrome://suitehelp/locale/help.rdf');"/>
> How are we going to handle all the existing openHelp calls?

From what I've seen, we need to add the URI to all of them, as the toolkit viewer doesn't know where to get its contents from if we don't tell it the URI (which is logical as it's a generic help viewer component)

Comment 13

[[e-mail]]

Comment on attachment 229072 [details] [diff] [review]
patch, v1.1: to use toolkit help viewer for suiterunner

(In reply to comment #12)
>>>+% content suitehelp %content/suitehelp/ xpcnativewrappers=yes
>>I am really not keen on the use of suitehelp as a package.
>>Can we not put the docs under chrome://communicator/locale/help/ ?
>Sure we can, I just though we might want to have it in a separate chrome
>package, but I'm OK with sticking it into communicator as well.
I don't see the point of creating a package just for some locale bits.

>>> 	typeaheadfind \
>But you didn't disable extensions/typeaheadfind. Do the two not conflict? Or
>> are you cheating by building toolkit chrome but overriding the component?
>Apparently they either don't conflict or override each other in a way that
>everything works. We really should disable extensions/typeaheadfind, I guess,
>but I didn't dare to risk this in the first patch, as we still can clean it up
>afterwards.
Well I double-checked and toolkit help requires toolkit nsITypeAheadFind which requires toolkit typeaheadfind. So for a start you will have to register it. Then you will need to disable the extensions version. And finally fix up all the other windows that use it...

>>>+                oncommand="openHelp('welcome', 'chrome://suitehelp/locale/help.rdf');"/>
>> How are we going to handle all the existing openHelp calls?
>From what I've seen, we need to add the URI to all of them, as the toolkit
>viewer doesn't know where to get its contents from if we don't tell it the URI
>(which is logical as it's a generic help viewer component)
Ok, but as this stands this will break xpfe help as this patch builds the docs under different URLs depending on MOZ_XUL_APP.

Comment 14

[[e-mail]]

You might find it easier to split the help docs move into a separate bug.

Comment 15

[Robert Kaiser]

(In reply to comment #14)
> You might find it easier to split the help docs move into a separate bug.

If you are OK with moving all the files listed in the cvs copy file here to suite/locales/en-US/common/help/ (translates to chrome://communicator/locale/help/ ) and leave them there as for-now-unused files that we can then patch and use when this bigger bug gets resolved, I'll just do a cvs copy bug and build upon those copied files afterwards right here in this bug.

About the typeaheadfind issue:
I wonder why both browser and help FAYT do work well with my build, but that may just be because suiterunner uses toolkit browser and tabbrowser right now.
What except browser is using XPFE typeaheadfind right now?

About openHelp():
I'm less concerned about breaking the openHelp() calls as I'd hope we can retire xpfe seamonkey ASAP - so either we break those calls until we flip the XOZ_XUL_APP switch or we fix them up only after we do that (and leave a bug open for that)

Comment 16

[[e-mail]]

(In reply to comment #15)
>If you are OK with moving all the files listed in the cvs copy file here to
>suite/locales/en-US/common/help/
I am.

>I wonder why both browser and help FAYT do work well with my build, but that
>may just be because suiterunner uses toolkit browser and tabbrowser right now.
Even so toolkit help uses findBar uses toolkit fayt (via browser).
So any patch would be incomplete unless fayt is [already] flipped.

>What except browser is using XPFE typeaheadfind right now?
>I'm less concerned about breaking the openHelp() calls as I'd hope we can
>retire xpfe seamonkey ASAP
Before we have a working suiterunner?

Comment 17

[Robert Kaiser]

(In reply to comment #16)
> >If you are OK with moving all the files listed in the cvs copy file here to
> >suite/locales/en-US/common/help/
> I am.

OK, cvs copy bug filed for that, see 344611

> >I wonder why both browser and help FAYT do work well with my build, but that
> >may just be because suiterunner uses toolkit browser and tabbrowser right now.
> Even so toolkit help uses findBar uses toolkit fayt (via browser).
> So any patch would be incomplete unless fayt is [already] flipped.

Hmm, I guess I'll have to look into that then... Just need to find out where else extensions/typeaheadfind is still being used - if anywhere...

> >I'm less concerned about breaking the openHelp() calls as I'd hope we can
> >retire xpfe seamonkey ASAP
> Before we have a working suiterunner?

No, but we should try to get it working as soon as we can...

But I guess we may better be able to live with keeping an open bug on changing over the openHelp() calls, which will be dependent on the "flip the switch" bug 328887

Comment 18

[[e-mail]]

(In reply to comment #17)
>But I guess we may better be able to live with keeping an open bug on changing
>over the openHelp() calls
So you're splitting this into two patches a) suiterunner should build toolkit help and typeaheadfind and b) switch all the openHelp calls?

Comment 19

[Robert Kaiser]

(In reply to comment #18)
> So you're splitting this into two patches a) suiterunner should build toolkit
> help and typeaheadfind and b) switch all the openHelp calls?

Yes, I think that's reasonable unless we want to introduce a big list of ifdefs or break xpfe SeaMonkey due to changed openHelp calls... we could introduce the second parameter as a no-op into the "old" openHelp() implementation though, in which case both SeaMonkey variants would work with the changed openHelp calls

Comment 20

[[e-mail]]

(In reply to comment #19)
>we could introduce the second parameter as a no-op into the "old" openHelp()
It already has the second parameter. The difference is that it has a default.

Comment 21

[Robert Kaiser]

(In reply to comment #20)
> (In reply to comment #19)
> >we could introduce the second parameter as a no-op into the "old" openHelp()
> It already has the second parameter. The difference is that it has a default.

Oh... I actually haven't checked if the toolkit one somehow supports a default - browser/ useage suggests it doesn't while mail/ useage suggests it may support that... I think I'll need to dig deeper there to find out about that...

Comment 22

[Robert Kaiser]

Attachment: use toolkit help viewer, don't build xpfe typeaheadfind

This patch is using the chrome://communicator/locale/help/ location, includes changes to incorporate what bug 343395 has done for security help buttons, and excludes extensions/typeaheadfind from the build.

With this applied, help works, even the overlayed security help buttons do work (using setHelpFileURI to set the help RDF file for all thoise openHelp calls) - and FAYT work in help (even though it displays a find bar).
What I couldn't find out yet is how to get FAYT in browser again via toolkit's typeaheadfind, but I figured a patch for that will likely touch a different set of files and can easily be split from this one - and I'm not yet sure if I'm able to find out how to do it myself, as I know almost nothing about FAYT (not even how it's invoked).

Comment 23

[Robert Kaiser]

Attachment: rename old typeaheadfind to be able to use it in parallel to toolkit (checked in)

Following Benjamin's suggestion, this is a patch to rename the old typeaheadfind component used by SeaMonkey to suitetypeaheadfind so that we can use it in parallel to toolkit's typeaheadfind component.

This approach removes the need of excluding extensions/typeaheadfind in the other patch posted here, and makes us able to handle bug 345526 (basing SeaMonkey's FAYT on toolkit code) independently of this bug here, as the old FAYT implementation still does work as it used to.

Comment 24

[Robert Kaiser]

Comment on attachment 230120 [details] [diff] [review]
use toolkit help viewer, don't build xpfe typeaheadfind

For reviewing the main patch, there are two small changes I've done locally, but I don't want to upload another patch just for those two lines:

>Index: mozilla/configure.in
>===================================================================
>+  if test "$MOZ_XUL_APP"; then
>+    MOZ_EXTENSIONS_DEFAULT=" wallet xml-rpc p3p venkman inspector irc spellcheck gnomevfs sroaming reporter"

should now be
+    MOZ_EXTENSIONS_DEFAULT=" wallet xml-rpc p3p venkman inspector irc typeaheadfind spellcheck gnomevfs sroaming 

along with the suitetypeaheadfind patch, of course.

>Index: mozilla/xpfe/communicator/resources/content/utilityOverlay.xul
>===================================================================
>+#ifdef MOZ_XUL_APP
>     <menupopup id="helpPopup">

move the #ifdef inside the <menupopup>, so that non-suiterunner builds still work.

Comment 25

[Benjamin Smedberg]

Comment on attachment 230250 [details] [diff] [review]
rename old typeaheadfind to be able to use it in parallel to toolkit (checked in)

I didn't look through the other patches on this bug, but you'll need to fix the packaging files in xpinstall/packager to reflect the name change.

Comment 26

[Robert Kaiser]

Attachment: update packaging files for suitetypeheadfind.xpt

Thanks for catching that, Benjamin. This hould go in along with the renaming patch.
I probably also should poke Camino devs as mozilla/camino/Camino.xcode/project.pbxproj refers to that .xpt as well.

Comment 27

[Robert Kaiser]

Attachment: update Camino for suitetypeheadfind.xpt

This should be all Camino needs for that file name change (I hope)...

Comment 28

[[e-mail]]

Comment on attachment 230915 [details] [diff] [review]
update packaging files for suitetypeheadfind.xpt

>-  deleteThisFile("Components", "typeaheadfind.xpt");
These lines are used during an in-place upgrade to remove obsolete or old versions of files, which you do want to do here. sr=me with this fixed.

Comment 29

[Robert Kaiser]

Attachment: update Camino for suitetypeheadfind (checked in)

Comment 30

[Robert Kaiser]

Attachment: update packaging files for suitetypeheadfind (include libs) (checked in)

I missed the libs in the first version this, here they should be included as well.

Comment 31

[[e-mail]]

Comment on attachment 230928 [details] [diff] [review]
update packaging files for suitetypeheadfind (include libs) (checked in)

r+sr=me with comment #28 changes as discussed on IRC.

Comment 32

[Robert Kaiser]

Comment on attachment 230250 [details] [diff] [review]
rename old typeaheadfind to be able to use it in parallel to toolkit (checked in)

checked in the bits to rename old typeaheadfind.

Comment 33

[Robert Kaiser]

Attachment: use toolkit help viewer, build toolkit typeaheadfind

This is an updated patch, corrected for the two changes I mentioned in comment #24, and taking into account that the help content files have already been cvs copied. Because of that, it includes correcting the stylesheet URL in the help content .xhtml files.

Comment 34

[Robert Kaiser]

There's one hunk I missed, please consider this part of that last patch:

Index: mozilla/suite/locales/en-US/chrome/common/help/helpFileLayout.css
===================================================================
 a[href^="http://"]:after, a[href^="https://"]:after
-{ content: url("chrome://help/locale/images/web-links.png"); }
+{ content: url("chrome://communicator/locale/help/images/web-links.png"); }

Comment 35

[Robert Kaiser]

Attachment: use toolkit help viewer, build toolkit typeaheadfind

Sorry, look like I'm good at bitrotting my own patches ;-)

Here's a patch that should apply correctly on current trunk, this also includes some previously missed conversions for help/locale to communicatior/locale/help in the new help locale files.

Comment 36

[[e-mail]]

Comment on attachment 231240 [details] [diff] [review]
use toolkit help viewer, build toolkit typeaheadfind

>Index: mozilla/suite/locales/en-US/chrome/common/help/suitehelp.rdf
Did you have to rewrite this file? How am I supposed to see what you changed?

>-  <a href="chrome://help/locale/help_help.xhtml">Using the Help Window</a>.</p>
>+  <a href="chrome://communicator/locale/help/help_help.xhtml">Using the Help Window</a>.</p>
This should be a relative link. In fact the stylesheets should be too.

Also I'll double-check but for some reason my help index was doubled.

Comment 37

[Robert Kaiser]

(In reply to comment #36)
> (From update of attachment 231240 [details] [diff] [review] [edit])
> >Index: mozilla/suite/locales/en-US/chrome/common/help/suitehelp.rdf
> Did you have to rewrite this file? How am I supposed to see what you changed?

I did rewrite it based on Firefox's file, it's hard to say what has changed.

> >-  <a href="chrome://help/locale/help_help.xhtml">Using the Help Window</a>.</p>
> >+  <a href="chrome://communicator/locale/help/help_help.xhtml">Using the Help Window</a>.</p>
> This should be a relative link. In fact the stylesheets should be too.

Hmm, you're right. Probably even the suite-toc.rdf entries, right?

> Also I'll double-check but for some reason my help index was doubled.

Usually, only the "Using Help" entry should be doubled. We still need to figure out how to handle that best, but this should be no blocker fo the moment, I'll file a followup on that.

Comment 38

[Stefan [:stefanh]]

Hmm, this patch doesn't seem to work on my mac (10.3.9) suiterunner debug build. Feels like some things are missing:

1) I got no Tools/Window menu. Or rather, no windows menu but a tools menu without any menu title in the menubar.

2) If I try to open Help, I get a parsing error:

3) Opening the "Go" menu results in a js error:
JavaScript error: chrome://global/content/hiddenWindow.xul, line 1: updateGoMenu is not defined

4) Opening the bookmarks menu also gives me a js error:
JavaScript error: chrome://navigator/content/navigator.js, line 1015: gBrowser has no properties

5) Selecting "Manage Bookmarks..." in the bm menu gives me this:
Bm menu --> Manage bookmarks:
JavaScript error: chrome://navigator/content/navigator.js, line 1357: toOpenWindowByType is not defined

Note:
This is with patched and clean source (clobber) - checkout finish: Tue Aug 1 17:53:32 CEST 2006

With clean source (clobber) - checkout finish: Wed Aug 2 00:35:29 CEST 2006 everything seems fine

Comment 39

[Stefan [:stefanh]]

(In reply to comment #38)

> With clean source (clobber) - checkout finish: Wed Aug 2 00:35:29 CEST 2006
> everything seems fine
> 

Erm, non-patched (of course)

Comment 40

[Robert Kaiser]

(In reply to comment #38)
> Hmm, this patch doesn't seem to work on my mac (10.3.9) suiterunner debug
> build. Feels like some things are missing:

I had such problems with an earlier revision of this patch when utilityOverlay had problems, but I fixed all those problem I saw in the current version of the patch...

Comment 41

[Robert Kaiser]

Attachment: build toolkit help viewer and typeaheadfind, hopefully work on Mac

This patch should hopefully work on Mac as well...

Comment 42

[Stefan [:stefanh]]

Comment on attachment 232979 [details] [diff] [review]
build toolkit help viewer and typeaheadfind, hopefully work on Mac

Sorry, but I don't see any difference with this. Fwiw, the parsing error I get when I try to open help is:

XML Parsing Error: undefined entity
Location: chrome://help/content/help.xul
Line Number 65, Column 5:    <key key="&goBackCmd.commandkey;" command="Help:Back" modifiers="accel"/>

Comment 43

[Robert Kaiser]

(In reply to comment #42)
> Sorry, but I don't see any difference with this.

I think you're not applying the configure.in patch to configure, and so you're ending up with duplicating help references (from suite/ and extensions/). So either you're patching configure or building with --enable-extensions=defaults,-help but else you can't accurately test this patch.

Comment 44

[Stefan [:stefanh]]

(In reply to comment #43)
> (In reply to comment #42)
> > Sorry, but I don't see any difference with this.
> 
> I think you're not applying the configure.in patch to configure, and so you're
> ending up with duplicating help references (from suite/ and extensions/). So
> either you're patching configure or building with
> --enable-extensions=defaults,-help but else you can't accurately test this
> patch.
> 
erm, you're right :-/

Comment 45

[Stefan [:stefanh]]

Comment on attachment 232979 [details] [diff] [review]
build toolkit help viewer and typeaheadfind, hopefully work on Mac

This works fine now on mac, so r=me for the mac part. Help opens, no regressions in Keyboard shortcuts, and FAT works as well.

Some general issues that should apply on all platforms (either fixing them now or file follow-up bugs):

1) We now use Firefox/Pinstripe navigation/print buttons in the Help toolbar. The same goes for the toolkit welcome page - that page is now styled differently than ours. We should of course use our own toolbar buttons and have the welcome page styled in our own way (even if we use the toolkit welcome page).


2) Opening Help produces a js error:
"JavaScript strict warning: chrome://global/content/bindings/tree.xml, line 135: reference to undefined property this.columns[i]"

(As a side-note, I tried the previous patch with a patched configure and the keyboard shortcuts didn't work.)

Comment 46

[[e-mail]]

(In reply to comment #45)
>2) Opening Help produces a js error:
>"JavaScript strict warning: chrome://global/content/bindings/tree.xml, line
>135: reference to undefined property this.columns[i]"
That's bug 332797.

Comment 47

[Robert Kaiser]

(In reply to comment #45)
> 1) We now use Firefox/Pinstripe navigation/print buttons in the Help toolbar.
> The same goes for the toolkit welcome page - that page is now styled
> differently than ours. We should of course use our own toolbar buttons and have
> the welcome page styled in our own way (even if we use the toolkit welcome
> page).

We should do that in a followup, probably using overlays or such.

> (As a side-note, I tried the previous patch with a patched configure and the
> keyboard shortcuts didn't work.)

Yes, I think the current one is better, as can support both Mac keyboard shortcuts.

Comment 48

[Stefan [:stefanh]]

(In reply to comment #47)
> (In reply to comment #45)
> > 1) We now use Firefox/Pinstripe navigation/print buttons in the Help toolbar.
> > The same goes for the toolkit welcome page - that page is now styled
> > differently than ours. We should of course use our own toolbar buttons and have
> > the welcome page styled in our own way (even if we use the toolkit welcome
> > page).
> 
> We should do that in a followup, probably using overlays or such.
>

Wouldn't everything be easier if Help was in chrome://help/locale/ ?

Comment 49

[Robert Kaiser]

(In reply to comment #48)
> Wouldn't everything be easier if Help was in chrome://help/locale/ ?

Overriding isn't much more work, actually, and chrome://help/ is reserved for the toolkit part, so it's no option. We can't pack stuff into toolkit.jar if we're serious with wanting to move to XULRunner later on.

Comment 50

[[e-mail]]

(In reply to comment #48)
>Wouldn't everything be easier if Help was in chrome://help/locale/ ?
In bug 346605 I'd like to move it all to chrome://communicator/locale/help although as yet I don't understand KaiRo's review comments...

Comment 51

[Robert Kaiser]

Attachment: build toolkit help viewer and typeaheadfind, updated to current trunk

This version is updated to current trunk after the recent changes done by Neil, the rest is still same as in the last patch.

Comment 52

[Robert Kaiser]

Attachment: better configure.in fix: remove old help from extensions list if building XUL_APP

Thanks to Neil's testing, we found out it's probably a good idea to remove the old xpfe help from the extensions list when building suiterunner, as its overlays clash with new UI pieces. This patch should do just that :)

Comment 53

[[e-mail]]

Comment on attachment 242167 [details] [diff] [review]
build toolkit help viewer and typeaheadfind, updated to current trunk

>+<!ENTITY % platformCommunicatorDTD SYSTEM "chrome://communicator-platform/locale/platformCommunicatorOverlay.dtd">
>+%platformCommunicatorDTD;
Unnecessary?

> 
>+  <commandset id="tasksCommands">
>+    <command id="cmd_openHelp"
>+             oncommand="openHelp('welcome', 'chrome://communicator/locale/help/suitehelp.rdf');"/>
>+  </commandset>
>+
Missing #ifdef MOZ_XUL_APP

Comment 54

[Stefan [:stefanh]]

(In reply to comment #53)
> (From update of attachment 242167 [details] [diff] [review] [edit])
> >+<!ENTITY % platformCommunicatorDTD SYSTEM "chrome://communicator-platform/locale/platformCommunicatorOverlay.dtd">
> >+%platformCommunicatorDTD;
> Unnecessary?

No, because in suite/locales/en-US/chrome/common/mac/platformCommunicatorOverlay.dtd#27:

 27 <!-- Help viewer -->
 28 <!ENTITY openHelpCmd.label          "&brandShortName; Help">
 29 <!ENTITY openHelpCmd.accesskey      "H">

Comment 55

[Robert Kaiser]

Comment on attachment 242241 [details] [diff] [review]
better configure.in fix: remove old help from extensions list if building XUL_APP

bsmedberg, could you please review this part?
I hope this is the correct way to
1) Have different default extensions for xpfe- and toolkit-based suite
2) exclude help extension if it might be included in the extension list when building toolkit-based suite (it's incompatible UI-wise)

Comment 56

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 242241 [details] [diff] [review]
better configure.in fix: remove old help from extensions list if building XUL_APP

how about:
  MOZ_EXTENSIONS_DEFAULT=" wallet xml-rpc p3p venkman inspector irc typeaheadfind gnomevfs sroaming reporter"
  if test -z  "$MOZ_XUL_APP"; then
    MOZ_EXTENSIONS_DEFAULT="$MOZ_EXTENSIONS_DEFAULT help"
  fi

Comment 57

[Robert Kaiser]

Checked in.

This should finally be fixed now. Further work on getting toolkit help perform correctly and look good enough in suiterunner all over the place should be done in followup bugs.

Comment 58

[[e-mail]]

Comment on attachment 242167 [details] [diff] [review]
build toolkit help viewer and typeaheadfind, updated to current trunk

>+                    <Description nc:panelid="search"
>+                        nc:datasources="search-db.rdf"
Except no such file exists, so we log two lines in the Error Console...