616193 - Trademark attribution for Charlton Company should only appear in en-US and en-GB

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Mike Beltzner [:beltzner, not reading bugmail]]

As per a recent update to our agreement with the Charlston Company, trademarkinfo.part2 only needs to appear in the en-US and en-GB builds. I suggest the easiest way to accomplish this would be for the other locales to leave the entity as an empty string.

Comment 1

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

If we don't want it localized, and only want it in specific locales, the most reliable solution is to not make it a localizable string, and insert it at run time based on a current-locale check.

Comment 2

[Axel Hecht [:Pike]]

To clarify, this does not affect en-ZA, and wouldn't affect en-CA if we had one?

Also, an en-US build with a language pack is not affected?

And yes, leaving it empty based on comments or bugs or somesuch is gonna be brittle.

Comment 3

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

I don't think the "theoretical-future en-* locales" or "en-US with a language pack" corner cases are particularly important, the main goal is to cover the en-* builds that we currently release.

I believe the patch that I'm going to attach will show the attribution text for our en-US and en-GB builds in their default state, or any other build with a en-GB or en-US language pack installed, which I think is sufficient.

Comment 4

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

Attachment: patch

Comment 5

[Axel Hecht [:Pike]]

Comment on attachment 494759 [details] [diff] [review]
patch

Yeah, sounds like the right concept. Should we landle license vs licence (en-GB)?

Comment 6

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

Don't think that's worth the effort.

Comment 7

[Justin Dolske [:Dolske]]

Comment on attachment 494759 [details] [diff] [review]
patch

>+++ b/browser/base/content/aboutDialog.js
...
>+#ifdef MOZ_OFFICIAL_BRANDING
>+  // Add Charlton trademark attribution, but only for en-US and en-GB

Johnath's original email on the subject suggested adding a "DO NOT REMOVE" comment, worth adding?

>+  let chromeRegistry = Cc["@mozilla.org/chrome/chrome-registry;1"].
>+                       getService(Ci.nsIXULChromeRegistry);
>+  let currentLocale = chromeRegistry.getSelectedLocale("global");
>+  if (currentLocale == "en-US" || currentLocale == "en-GB") {
>+    document.getElementById("extra-trademark").value = "Some of the trademarks used under license from The Charlton Company.";
>+  }

How about making this more idiot-proof by placing the text into the XUL, and setting .hidden = true here (when it's not en-US / en-GB)? Pehaps I'm just being overly paranoid. :)


>+++ b/other-licenses/branding/firefox/locales/en-US/brand.dtd
>@@ -1,5 +1,4 @@
> <!ENTITY  trademarkInfo.part1   "Firefox and the Firefox logos are trademarks of the Mozilla Foundation.">
>-<!ENTITY  trademarkInfo.part2   "Some of the trademarks used under license from The Charlton Company.">

Change the entity name for .part1? Could be a locale that swapped the order of the two parts, thinking they're a single-but-split unit, and so this would result in dropping the MoFo trademark notice?

Or is it assumed that when .part2 is removed, the l10n tools will alert on that and localizers will fix any .part1 issue?

Comment 8

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

(In reply to comment #7)
> Change the entity name for .part1? Could be a locale that swapped the order of
> the two parts, thinking they're a single-but-split unit, and so this would
> result in dropping the MoFo trademark notice?

I think the odds of a locale having switched the order is pretty low, and I fear that making this a more complicated change than just a string removal will actually increase the possibility of something being handled wrong (more involved change for localizers -> more error-prone). Would be interested in Axel's opinion on this I guess.

Comment 9

[Axel Hecht [:Pike]]

I've looked at mxr, and there are only 4 locales where it's not apparent that part2 talks about Charlton.

That's ar, pa-IN, ml, te, let's just get bugs on file to verify for those.

Comment 10

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

Attachment: updated patch

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/be63daa6e0f8

Comment 12

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

(In reply to comment #9)
> That's ar, pa-IN, ml, te, let's just get bugs on file to verify for those.

Filed those (see dependencies).

Comment 13

[u60234]

Building with official branding doesn't remove the Charlton trademark attribution for sv-SE. Error console reports that "Cc is not defined" in aboutDialog.js.

Changing Cc to Components.classes, and Ci to Components.interfaces, removes the extra-trademark label.

Comment 14

[u60234]

Attachment: About dialog with English text at the bottom

Comment 15

[Johnathan Nightingale [:johnath]]

Attachment: Add Cc, Ci

Was surprised to hear this, since it pulls in Services.jsm, but then I realized that it doesn't export Cc, Ci.

Comment 16

[Justin Dolske [:Dolske]]

Comment on attachment 504492 [details] [diff] [review]
Add Cc, Ci

(In reply to comment #7)

> How about making this more idiot-proof by placing the text into the XUL, and
> setting .hidden = true here (when it's not en-US / en-GB)? Pehaps I'm just
> being overly paranoid. :)

Apparently I am not overly paranoid. ;)

Tested patch, confirmed it fixes the problem.

Comment 17

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

Attachment: avoid using Cc/Ci

The reason I didn't catch this is that I tested on Mac, where this works as-is (presumably because of the menu overlay). Oddly enough, that patch doesn't seem to break things on Mac, though, which I don't quite understand (I would expect a redeclaration error). To play it safe, I think we're better off just not relying on the shorthands.

Comment 18

[:Margaret Leibovic]

Pushed fix:

http://hg.mozilla.org/mozilla-central/rev/6cee9f946cbe