When I looked at the patches for bug 328563, I realized once again that toolkit's about: page currently forces all apps using it into naming their release notes <version>.html on the web server. The place where this is enforced is http://lxr.mozilla.org/mozilla/source/toolkit/content/about.xhtml#71 - it links to "&releaseBaseURL;__MOZ_APP_VERSION__.html" for the relnotes. This is bad, as some apps probly want to follow other conventions on their web pages, e.g. SeaMonkey, starting to use toolkit on trunk, has relnotes at http://www.mozilla.org/projects/seamonkey/releases/seamonkey<version>/ - and that doesn't fit with toolkit's about page. I think probably the best solution here would be to link to "&releaseURL;" in about.xhtml and preprocess brand.dtd instead, adding the version number there, as the branding files are app-specific and can use their specific URL convention. That said, we should really have some place to file such bugs in the Toolkit product instead of Firefox.

AFAIK there's a convention to not preprocess locale files though. Maybe you can use a pref instead. But please don't hardcode the version again.

True, we have this convention, and it's quite good, I'm a big fan of that convention. But I think that we can break it in very controlled areas, which I'd suppose includes those branding files.

Any app-specific changes should be made in chrome://branding/content/brand.dtd, not chrome://branding/locale/brand.dtd

Benjamin, that would need a toolkit-based app cpouldn't allow for localized relnotes (or at least not if they need a different URL)?

OK, so after some discussion with Pike on IRC, I understand we could/should do it the following way: 1) include both locale/brand.dtd and content/brand.dtd from about.xhtml, in this order. 2) content could decide to either pull parts or complete urls from locale, or to "hardcode" the entity value for that URL I may look into doing this, when I find the time to do so. My target is to have this working in some fashion before SeaMonkey flips to toolkit on trunk.

I know this is XHTML and not JS itself, but can we utilize the new url formatter service for this? As I'm told that the URL formatter service also allows pulling its URL through a localized pref, I think we can resolve all issues that way (and use one source for the relnotes URL everywhere probably).

Dietrich: will this be fixed in the last of your series of patches to bug 348076? Is this dialog changed so that it uses the new pref that you created there?

(In reply to comment #7) > Dietrich: will this be fixed in the last of your series of patches to bug > 348076? Is this dialog changed so that it uses the new pref that you created > there? > Yes.

The fix for this should address Mark's comments from bug 348076: (In reply to comment #74) > (From update of attachment 234632 [details] [diff] [review] [edit]) > I've also just remembered a couple of comments on the about.xhtml page: > > + var releaseNotesURL = formatURL("app.releaseNotesURL", null, true); > ... > + function formatURL(aFormat, aVars, aIsPref) { > + var repl = null; > + if (aVars) { > + repl = > Cc["@mozilla.org/dictionary;1"].createInstance(Ci.nsIDictionary); > + for (var key in aVars) { > + var val = > Cc['@mozilla.org/supports-string;1'].createInstance(Ci.nsISupportsString); > + val.data = aVars[key]; > + repl.setValue(key, val); > + } > + } > > Cc and Ci aren't defined for non-firefox applications - I think they are > defined in browser.js which is specific to ff. > > Also, as this function is always called with aVars null, and aIsPref true, do > you really need to use the formatURL function in this instance? >

Fixes and simplification as described in comment #9.

Comment on attachment 235342 [details] [diff] [review] cleanup as noted in comment #9 r=me

Between the changes in bug 348076 and the attached patch, we've met the desired level of flexibility.

Comment on attachment 235342 [details] [diff] [review] cleanup as noted in comment #9 Drivers: This patch fixes the "release notes" link in about: (currently broken) and allows non-Fx apps to benefit from the changes (some current code is Fx-only).

Will this fix "Error: uncaught exception: Permission denied to get property UnnamedClass.classes"?

(In reply to comment #14) > Will this fix "Error: uncaught exception: Permission denied to get property > UnnamedClass.classes"? I believe a fix is required for bug 349985 to fix that error.

Comment on attachment 235342 [details] [diff] [review] cleanup as noted in comment #9 a=schrep for drivers

Checking in about.xhtml; /cvsroot/mozilla/toolkit/content/about.xhtml,v <-- about.xhtml new revision: 1.8.8.10; previous revision: 1.8.8.9 done