359174 - Move a number of SeaMonkey URLs to branding

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Robert Kaiser]

We have a few URLs that are going through URL formatter now, and we should make those localized prefs, so that localizers can possibly redirect them to a localized resource where one exists (or otherwise leave them original).

The correct place to do this seems to be brand.properties (we also could create a urls.properties in branding, but brand.properties isn't big and already there).

I'll attach a patch in a minute.

Comment 1

[Robert Kaiser]

Attachment: get localized pref URLs into brand.properties (checked in)

This patch moves the URLs for "get new themes" and "get new extensions" to SeaMonkey-specific localized prefs (the toolkit people intend to remove the definitions from extensions.properties in the future) - those are suitrunner-only, it also moves the dictionaries URL to brand.properties and adds a release notes URL there that is used with toolkit's about: window (once that one has permissions to call urlformatter) and can used by all other SeaMonkey code that refers to release notes in the future, given that it's passed through urlformatter.

Once this is in, we can work on using those dictionaries and release notes URLs through urlformatter in other places (mailnews start page, help menu, etc.)

Comment 2

[Robert Kaiser]

Comment on attachment 244426 [details] [diff] [review]
get localized pref URLs into brand.properties (checked in)

>Index: mozilla/editor/ui/locales/en-US/chrome/region/region.properties
>===================================================================
>-editor.spellcheckers.url=https://addons.mozilla.org/search.php?cat=68&app=seamonkey&type=E

Hrmm, we can't do this yet, as this is still used:
http://lxr.mozilla.org/mozilla/search?string=editor.spellcheckers.url

We should look into cleaning those up and use urlformatter there.

For now, ignore this hunk in reviews, the rest of this is orthogonal to this.

Comment 3

[Robert Kaiser]

Attachment: use urlformatted dictionary URL everywhere (checked in)

This patch makes us use the urlformatted dictionary URL in all places where we still used the old one, and removes the old one from editor.

Comment 4

[Robert Kaiser]

Attachment: use urlformatted relnotes URL (help part not working yet)

I'm attaching this patch although it's not working as expected: This should move us to using the urlformatted relnotes URL everywhere (in suiterunner, xpfe-based suite still has old reference in about.xhtml but I won't touch that).

Unfortunately, the welcome_help.xhtml part doesn't work (yet), as it's a locale document and those seem not to be priviledged enough, the |dump(ex);| statement dumps the following error to my console window:
Permission denied to get property XPCComponents.classes

Comment 5

[Robert Kaiser]

Attachment: use urlformatted relnotes URL and start suite helpOverlay

Pointing relnotes to the urlformatted version wasn't as easy as I had hoped due to the help stuff. Utilizing the click interception infrastructure of bug 226911 was nicely - but that was introduced after forking toolkit help viewer and never ported to it.
It may be a good idea for toolkit to pick this up, but until they may decide to do so, I've bitten the bullet and introduced our helpOverlay that we'll probably need for stuff like index and glossary anyways.
That way, I've introduced a generic way for help documents to use url prefs going through urlformatter, just note <a href="urlpref:my.pref.name"> and this will call the formatted URL specified by the pref "my.pref.name" - and show it to the user as if it was a normal external URL in the help file.

Comment 6

[[e-mail]]

Comment on attachment 244446 [details] [diff] [review]
use urlformatted dictionary URL everywhere (checked in)

>+<script type="application/x-javascript">
>+  // get release notes URL from prefs
>+  try {
>+    var formatter = Components.classes["@mozilla.org/toolkit/URLFormatterService;1"]
>+                              .getService(Components.interfaces.nsIURLFormatter);
>+    var dictURL = formatter.formatURLPref("spellchecker.dictionaries.download.url");
>+    var relnotes = document.getElementById("dictURL");
>+    relnotes.setAttribute("href", dictURL);
>+
>+    var releaseNotesURL = formatter.formatURLPref("app.releaseNotesURL");
>+    var relnotes = document.getElementById("releaseNotesURL");
>+    relnotes.setAttribute("href", releaseNotesURL);
>+  }
>+  catch (ex) { dump(ex); }
Nit: duplicate declaration of var relnotes (the first is probably wrong).
The URL formatter doesn't throw on error (it just returns about:blank) but you might want to null-check relnotes rather than blindly using try/catch.

>+<a id="dictURL" href="">dictionaries</a> section on 
Nit: trailing space. Also, href="" should be unnecessary.

Bah, amo is redirecting us to "Fx dictionaries" :-(

Comment 7

[Robert Kaiser]

(In reply to comment #6)
> Nit: duplicate declaration of var relnotes (the first is probably wrong).
> The URL formatter doesn't throw on error (it just returns about:blank) but you
> might want to null-check relnotes rather than blindly using try/catch.

Fixed first var to "dictionaries" locally and removed the try/catch as it probably is useless in this case (we probably want to see a possble error easily when getting the urlformatter), but I added a |if (relnotes)| line before the setAttribute instead to catch localizers not preserving the ID correctly (or leaving it out for some good reason).

> >+<a id="dictURL" href="">dictionaries</a> section on 
> Nit: trailing space. Also, href="" should be unnecessary.

href="" is needed to be valid XHTML, but I fixed the trailing space.

> Bah, amo is redirecting us to "Fx dictionaries" :-(

Yes, the redirect isn't completely correct, but the dictionaries there are at least those that work for us.

Gotta wait for an open tree to check in anyways...

Comment 8

[[e-mail]]

Comment on attachment 244523 [details] [diff] [review]
use urlformatted relnotes URL and start suite helpOverlay

>+var formatter;
No point making the formatter a global. Just fetch it when you need it.

>+  var loadURI = target.href;
That's a confusing name... uri or href will do.

>+  if (loadURI.lastIndexOf("urlpref:", 0) == 0) {
The bogus scheme needs to be more bogus, such as x-moz-url-link:

>+      loadURI = formatter.formatURLPref(loadURI.substr(8));
Magic number ;-) Maybe use /^x-moz-url-link:/.test and RegExp.rightContext?

>+    var formatter = Components.classes["@mozilla.org/toolkit/URLFormatterService;1"]
>+                              .getService(Components.interfaces.nsIURLFormatter);
>+    var releaseNotesURL = formatter.formatURLPref("app.releaseNotesURL");
>+    openTopWin(releaseNotesURL);
Possibly combine these into one statement?

>+<!DOCTYPE overlay [
>+
>+<!ENTITY % helpOverlayDTD SYSTEM "chrome://communicator/locale/helpOverlay.dtd">
>+%helpOverlayDTD;
>+
>+]>
Use <!DOCTYPE overlay> and add the DTD if and when you actually need it.
r+sr=me with the DTD removed from the patch.

Comment 9

[Robert Kaiser]

Checked in with the requested changes.