366814 - Link in about: should be urlformatted localized pref

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Bruno 'Aqualon' Escherl]

Like KaiRo suggested in bug 366367 comment 1 the link in http://lxr.mozilla.org/seamonkey/source/toolkit/content/about.xhtml#63 should also be a localized pref (suggested name is app.vendorURL).

Comment 1

[Bruno 'Aqualon' Escherl]

Attachment: Use app.vendorURL for link in aboutLogoContainer

This depends on bug 349985 like showing releaseNotesURL does. So far mozilla.org should be the default. Firefox and Thunderbird will need their own patches to get app.vendorURL as prefs, SeaMonkey suiterunner gets it when bug 366367 is resolved.

Comment 2

[Robert Kaiser]

Hmm, actually this can very well land without bug 349985 being fixed, as the intelligent solution to have mozilla.org in by default means that behavior stays as it is currently while urlformatter is unavailable to us, and we magically start pointing to the right vendorURL once that get resolved :)

mconnor, any chance you get around to reviewing this?

Comment 3

[Reed Loden [:reed]]

Attachment: patch to check-in (unbitrotted)

Unbitrotten version for check-in. Carrying over r+.

Comment 4

[Reed Loden [:reed]]

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

Comment 5

[Axel Hecht [:Pike]]

AFAICT, Firefox doesn't have app.vendorURL, and will point to about:blank then.

Reopening, this shouldn't have a chance to regress firefox.

Comment 6

[Reed Loden [:reed]]

(In reply to comment #5)
> AFAICT, Firefox doesn't have app.vendorURL, and will point to about:blank then.

This WFM on latest nightly:
Build identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a8pre) Gecko/2007081404 Minefield/3.0a8pre

Is it not working for you? Please note that the vendorURL entity has a default value, so that value seems to be used.

Comment 7

[Axel Hecht [:Pike]]

As comment 2 notes, the urlformatter isn't even used at all 'til bug 349985 is fixed. Thus, any app uses the default. Once the urlformatter kicks in, it will not fault, but will return the default of the urlformatter, which is about:blank.

We should either:

- back this out
- have someone in charge to add the pref. I'm curious where that should be, Benjamin? Maybe even just put this in all.js.
- fix the code to not overwrite the default when the urlformatter returns about:blank (Not so good, imho)

Comment 8

[Robert Kaiser]

Sure the default value is used as bug 349985 prevents us from even fetching vendorURL from prefs atm.

Anyways, Axel, I had hoped Firefox probably will fall back even once that bug is fixed and we could fetch it. The logical thing would have been for the urlformatter to error out/throw when not finding the pref to work on - and in this case such a fallback would work nicely.
Still Firefox may even want to set it explicitly to point to mozilla.com instead of mozilla.org, IMHO.

Oh, and http://mxr.mozilla.org/seamonkey/source/toolkit/locales/en-US/chrome/global-region/region.dtd#2 is probably obsolete (just found this when looking where it's defined).

I'm heavily against backing this out though, as the toolkit part done here is the correct thing to do and if some consumer of toolkit hasn't caught up (even if it's the probably most important consumer right now), that's no good argument for crippling toolkit - esp. as the fix is that easy and it doesn't even violate any freezes or such.

Comment 9

[Reed Loden [:reed]]

(In reply to comment #7)
> - have someone in charge to add the pref. I'm curious where that should be,
> Benjamin? Maybe even just put this in all.js.
> - fix the code to not overwrite the default when the urlformatter returns
> about:blank (Not so good, imho)

I can do either of these, depending on what Benjamin wants. I don't think we should back out, as it would hurt the users of app.vendorURL. Just seems easier to fix the issue than to back it out. :)

Comment 10

[Axel Hecht [:Pike]]

Re comment 8, http://lxr.mozilla.org/mozilla/source/toolkit/components/urlformatter/src/nsURLFormatter.js#128 is the code that catches non-existing prefs and it returns about:blank in that case.

Comment 11

[Axel Hecht [:Pike]]

Yikes, we never did anything about this code.

Just filed bug 550870 to end up with less localizable URLs, too.

Comment 12

[Robert Kaiser]

(In reply to comment #11)
> Yikes, we never did anything about this code.

True.

> Just filed bug 550870 to end up with less localizable URLs, too.

None of those mentioned there touches this bug.

Comment 13

[Bruno 'Aqualon' Escherl]

After bug 349985 is finally fixed, this can be resolved fixed again :)

Comment 14

[:Gavin Sharp [email: [e-mail]]]

The link in about: is still broken in Firefox. I filed bug 606289.