456439 - add about:rights and a "Know Your Rights" infobar to Firefox

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Kev Needham [:kev]]

Attachment: Draft about:rights text (v0.1)

add an about:rights resource which will display the attached text in the current browser window. please note that the text is draft, requires minor formatting changes to work with chrome://global/skin/about.css, and will need to be localized. the "Mozilla Web Site Services" (currently a header with an id of "websiteservices") should be a clickable link which expands the ordered list below it (id="websiteservices-terms"), which should be collapsed by default.

additionally, add an infobar that appears on firstrun, or the first time firefox is run after the about:rights text changes. the infobar should be similar to the "Your Rights Infobar" mockup attached to this bug. clicking on the "Know Your Rights..." button would open "about:rights" in a new tab.

Comment 1

[Kev Needham [:kev]]

Attachment: Know Your Rights infobar mockup

Comment 2

[Kev Needham [:kev]]

Attachment: Final about:rights text (v1)

updated version of about:rights text. should be final. minor formatting change, minor revision to para on optional services. text will still need to be formatted for use as an about: dialog, and have the collapsing text js added.

Comment 3

[Jesse Ruderman]

I like the idea of having an about:rights for seeing the the agreement, but I don't think I like the infobar idea.  When would it appear and when would it disappear?

Comment 4

[Kohei Yoshino [:kohei]]

The term "clicking Preferences -> Security" is Mac-specific, should be "opening Options/Preferences window, selecting Security tab."

BTW, do we need a legal review for the translation?

Comment 5

[Daniel Veditz [:dveditz]]

I don't think we can "block" a specific security release for this. When it's done and tested (landed for 3.1) we can approve it.

Comment 6

[Justin Dolske [:Dolske]]

(In reply to comment #2)
> Created an attachment (id=340238) [details]
> Final about:rights text (v1)

Can we link to http://www.mozilla.org/MPL/ for the MPL, instead of the actual license text? I think it's better than sending the user directly into a blob of legalese. [That page could use a bit of cleanup/focus, but the goal here is to link to something readable to set up a context/overview of the MPL. The "EULA" bits on that page probably need revised now too. Maybe this should just be a new mozilla.com page specific to this purpose.]

Comment 7

[Justin Dolske [:Dolske]]

Attachment: Patch v.1 (WIP)

This patch implements an "about:rights" page (nothing else yet).

* It would be nice to get some styling on the page so it's not a black and white ugly block of text. I'll ping some of the UX folk to see what we can do.

* How should localization be handled for the links on the page? I put the links in the DTDs to localizers can adjust them if needed, but if they're going to be localized server side w/redirects (or not localized at all), perhaps they should be static.

* I made a best guess at how to handle branding in the text. Most things in the browser code use replaceable strings for things like the vendor name and product name... We use "Mozilla" and "Firefox" (respectively) for releases, "Mozilla" / "Minefield" for nightly and personal builds (ie, the default), and "mozilla.org" / "Shiretoko" for alpha builds. So, the interesting question here is what the about:rights page should be saying in these cases. I went ahead and used the replaceable strings, which means the page says slightly different things depending on how the code is built. Maybe that's wrong, and the whole page should be hardcoded to Mozilla/Firefox?

[Of course, "Mozilla Public License" is always static. I also left "Mozilla does not grant you any rights to the Mozilla and Firefox trademarks or logos." static, since the I think the intent there is specific to "Mozilla" and "Firefox" (no matter how you're building it).]

Comment 8

[Reed Loden [:reed]]

Comment on attachment 343497 [details] [diff] [review]
Patch v.1 (WIP)

>+<!ENTITY rights.intro-point3link "http://www.mozilla.com/en-US/legal/privacy/firefox-en.html">

Drop the "en-US/" from this URL, please.


Will this need to go through security review since it's another about:, and it permits script?

Comment 9

[Kev Needham [:kev]]

Attachment: about:rights text with formatting

Here's a formatted version of the text, which can be used as a base.

Comment 10

[Kev Needham [:kev]]

meant to add, the xhtml file is pretty much what's being used by ubuntu right now.

Comment 11

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

I spoke with Sam about this yesterday, and will accept a quantum of fail here as I don't think I ever flagged this as a potential blocker, but we should be looking to get this issue resolved in a dot release as soon as we can. I don't think I fully realized that we were so close to the 1.9.0.4 deadline here, though, and it feels a lot late to do the localization work (especially with the 3.1 string freeze coming up). Nominating just in case there are other things that might push the release out, though :)

OTOH, this definitely needs to be in for 1.9.1 and 1.9.0.5, which means we need to think about:

 - localization requirements
 - security review requirements (unless we can inherit that from the about:robots work)

Comment 12

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

Urf, I just realized that this is a 3.1 blocker and we're coming up on string freeze, so we definitely need answers about the localization requirements (who can translate? who needs to review translations?) post haste!

Comment 13

[Kev Needham [:kev]]

Attachment: about:rights text with formatting (linux-i686)

renamed "hiddenpart" id and references to "websiteservices".

linux-specific menu items verified in text of how to disable SafeBrowsing.

Comment 15

[Daniel Veditz [:dveditz]]

Sorry, but I don't see how this can possibly be done by Oct 24 in time for 1.9.0.4 -- next release it is.

Comment 16

[Justin Dolske [:Dolske]]

Attachment: Patch v.2

* I'm not 100% sure where the notification bar string should live. I don't think we want a 3rd party product to be displaying "RoncoBrowser 3000 is free and open software from the non-profit Mozilla Foundation" (and neither do they!). Maybe this string should move to brand.properties, which a 3rd party would presumably have to edit anyway?

* The meaning of browser.EULA.override aka browser.rights.override changes slightly from the way bug 443918 recently added it. It now works as, well, a real override -- when set, it explicitly forces the bar to either be shown or not shown, unconditionally. Previously it was being used to both force never showing a EULA (for test box usage) and to specify that the EULA should be shown by default (but we might not, if you had already seen it). Comments 3+4 in that bug indicate that the goal was to only show a EULA in official releases; so I changed the default behavior to not show the about:rights notification unless it's (1) it's an official build and (2) hasn't already been shown.

Comment 17

[Justin Dolske [:Dolske]]

Attachment: Screenshot of what startup would look like

Comment 18

[Justin Dolske [:Dolske]]

Attachment: Screenshot of about:rights page

I feel like a n00b, posting an image of text, but otherwise it's hard to see the full format and style from just the code. :-) Note that this page picks up branding, so an official release won't actually say "Minefield". I should probably have spun up a build with official branding for the screenshot... oh well.

Oh, I should also note that the patch has another slight change to the text; specifically the last sentence in the first paragraph under "Minefield Web Site Services". The instructions for getting to the preferences would be platform-dependent, and that's a pain to do. So I adjusted the text to be platform agnostic, and the cost of not being quite as explicit.

Comment 19

[Johnathan Nightingale [:johnath]]

Comment on attachment 344564 [details] [diff] [review]
Patch v.2

A couple of small drive-bys only because I love.

>diff --git a/browser/base/content/aboutRights.xhtml b/browser/base/content/aboutRights.xhtml
>new file mode 100644
>--- /dev/null
>+++ b/browser/base/content/aboutRights.xhtml
>@@ -0,0 +1,98 @@
>+<?xml version="1.0" encoding="UTF-8"?>
>+<!DOCTYPE html [
>+  <!ENTITY % htmlDTD PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "DTD/xhtml1-strict.dtd">
>+  %htmlDTD;
>+  <!ENTITY % globalDTD SYSTEM "chrome://global/locale/global.dtd">
>+  %globalDTD;
>+  <!ENTITY % brandDTD SYSTEM "chrome://branding/locale/brand.dtd">
>+  %brandDTD;
>+  <!ENTITY % securityPrefsDTD SYSTEM "chrome://browser/locale/preferences/security.dtd">
>+  %securityPrefsDTD;
>+  <!ENTITY % aboutRightsDTD SYSTEM "chrome://browser/locale/aboutRights.dtd">
>+  %aboutRightsDTD;
>+]>

I suspect that we don't need preferences/security.dtd here?

>diff --git a/browser/components/aboutRights.js b/browser/components/aboutRights.js
>new file mode 100644
>--- /dev/null
>+++ b/browser/components/aboutRights.js
>+  getURIFlags: function(aURI) {
>+    return (Ci.nsIAboutModule.ALLOW_SCRIPT |
>+            Ci.nsIAboutModule.URI_SAFE_FOR_UNTRUSTED_CONTENT);
>+  },

Is it our intent to have web content be able to link to about:rights?  I don't know if there's any problem with letting it happen, I think probably not, but I wanted to make sure it was intentional.

>+  _shouldShowRights : function () {
>+    // Look for an unconditional override pref. If set, do what it says.
>+    // (true --> never show, false --> always show)
>+    var shouldShow;
>+    try {
>+      shouldShow = !this._prefs.getBoolPref("browser.rights.override");
>+      return shouldShow;
>+    } catch (e) { }

Do we need shouldShow in here at all?  Can't we just return in each try, and if the call fails, we'll jump to the catch, and fall through to the next step?  Feels like it might be more compact & readable that way, but I admit it's a stylistic flourish more than a correctness issue.

Comment 20

[Justin Dolske [:Dolske]]

(In reply to comment #19)

> >+++ b/browser/base/content/aboutRights.xhtml
>
> I suspect that we don't need preferences/security.dtd here?

We do, it's used in aboutRights.dtd for referring to the UI text for disabling safebrowsing. The strings already changed between FF3 and F3.1, so I figured I'd make it prepared for another change.

It kind of sucks either way... If I hard code the strings here, there's risk of going stale (when the prefs UI changes but about:rights is forgotten). If I import the DTD, there's risk of breaking about:rights if those entity names change. [Perhaps a simple mochitest to both ensure about:rights loads and has expected structure would help...]

> >+++ b/browser/components/aboutRights.js
> >+  getURIFlags: function(aURI) {
> >+    return (Ci.nsIAboutModule.ALLOW_SCRIPT |
> >+            Ci.nsIAboutModule.URI_SAFE_FOR_UNTRUSTED_CONTENT);
> >+  },
> 
> Is it our intent to have web content be able to link to about:rights?

Not directly, it's just a side effect of the wonky way about: modules work. In order to drop chrome privs, you end up making it linkable.

> >+    var shouldShow;
> >+    try {
> >+      shouldShow = !this._prefs.getBoolPref("browser.rights.override");
> >+      return shouldShow;
> >+    } catch (e) { }
> 
> Do we need shouldShow in here at all?  Can't we just return in each try

We could. I felt less bad about the try-catch bracket soup with two lines instead of one. :)

Comment 21

[Johnathan Nightingale [:johnath]]

(In reply to comment #20)

> > Is it our intent to have web content be able to link to about:rights?
> 
> Not directly, it's just a side effect of the wonky way about: modules work. In
> order to drop chrome privs, you end up making it linkable.

Hmm - shouldn't the code in newChannel do that?  I only mentioned this in the first place because I made the opposite mistake once, assuming that URI_SAFE would *make* it safe.  So then I implemented newChannel and all was well for me, but it left me with lingering doubt as to the nature of the flag, which I really think only designates the page as linkable. The IDL comment leaves it kind of ambiguous.  Gavin points out that, for about pages implemented in nsAboutRedirector, that flag is also used to trigger the change in principal that you're doing in newchannel.

Honestly, in this case the more I think about it, the more I think it's a good thing.  Someone will blog about it, link to it, and won't it be nice that that link works?

> > Do we need shouldShow in here at all?  Can't we just return in each try
> 
> We could. I felt less bad about the try-catch bracket soup with two lines
> instead of one. :)

:)  Fair enough.

Comment 22

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

Comment on attachment 344564 [details] [diff] [review]
Patch v.2

Only a couple of nits:

 - the "web site services" section should be collapseable

 - there should be no ":" in "About: Your Rights"

I think Kev has code for the first aspect as well as some other styling nits. Other than that, this is good by me.

I'm checking with Harvey to make sure we don't want to show the infobar even for unbranded builds. I think we might want to, as now this is more about the user's rights than our liabilities ;)

Comment 23

[Justin Dolske [:Dolske]]

(In reply to comment #22)
> Only a couple of nits:
> 
>  - the "web site services" section should be collapseable

It is, the screenshot just shows it expanded.

Comment 24

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

Comment on attachment 344564 [details] [diff] [review]
Patch v.2

>+<!ENTITY rights.intro-point3link "http://www.mozilla.com//legal/privacy/firefox-en.html">

I think this should be /legal/privacy to make it clear that the privacy policy is only for Firefox (as per the text of that policy). We might also want to change the bullet to be less specific, as Minefield doesn't actually have a privacy policy. Something like:

+<!ENTITY rights.intro-point3a "Privacy policies for &vendorShortName;'s products may be found ">

Comment 25

[Reed Loden [:reed]]

(In reply to comment #24)
Beltzner's correct. Thanks for catching that, beltzner. :)

Comment 26

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

Harvey, Kev and I just had a good conversation about whether or not this should be included in unbranded as well as branded builds. Some background context:

 - reducing the differences between unbranded (Minefield) and branded (Firefox) builds is good for testing and ensuring that we know what we're shipping

 - some of the rights we afford users are not guaranteed in unbranded (Minefield) builds; for example, as mentioned in comment 24, Mozilla Corporation has no privacy policy for Minefield as it doesn't really *own* the product per se nor is guaranteed to be the sole distributor

 - the website services agreement terms are only made between Mozilla Corporation and partners for official, branded (Firefox) versions; while in many cases the services will still work with the unbranded (Minefield) user-agent string, our partners are under no obligation for that to be the case (they do so for testing reasons, same reasons that we invest so heavily in the unbranded version)

The conclusion we came to was: we absolutely must ship the infobar and about:rights page in unbranded versions, but the content should be different and applicable to that unbranded version. Here's what we think it should look like:

-----------------

About Your Rights

&productName; is free and open-source software, built by a community of thousands from all over the world. There are a few things you should know:

* &productName; is made available to you under the terms of the _Mozilla Public License_. This means you may use, copy and distribute &productName; to others. You are also welcome to modify the source code of &productName; as you want to meet your needs. The Mozilla Public License also gives you the right to distribute your modified versions.

* Any applicable privacy policies for this product should be listed here.

* &productName; also offers optional web site information services, such as the SafeBrowsing service; however we cannot guarantee that they are 100% accurate or error-free. More details, including how to disable these services, can be found in the _service terms_.

&productName; Web Site Services

&productName; uses web site information services ("Services") such as the SafeBrowsing service, that are available for your use with this binary version of &productName; as described below. If you do not want to use the Services or the terms below are unacceptable, you may disable the SafeBrowsing service by opening the application preferences, selecting the security section, and unchecking the options for "Block reported attack sites" and "Block reported web forgeries"

* Any applicable service terms for this product should be listed here.

-----------------

The goal here is to show the template that's used for the about:rights page, so that other distributors (such as ourselves) can build off of it.

Comment 27

[Justin Dolske [:Dolske]]

Attachment: Patch v.3

Updated with beltzner's nits.

I'm having trouble getting the "branded only" bits to turn on in my build environment, apparently --enable-official-branding isn't enough and BUILD_OFFICIAL or MOZILLA_OFFICIAL needs to be set in the environment (my clobber build is running now, which will hopefully pick that up finally). This code is used in some existing Makefiles, so I assume I'm just having problems getting my build environment right.

Comment 28

[Justin Dolske [:Dolske]]

Attachment: Screenshot of unbranded about:rights page

Comment 29

[Justin Dolske [:Dolske]]

Attachment: Screenshot of branded about:rights page

(Got my branded environment working)

Comment 30

[Axel Hecht [:Pike]]

L10n comments, sorry for this coming in so late. Just tiny nits though.

For RTL, please add an entity for that with a localization note. If a RTL would choose to not dive into translating legal, the English text should display LTR.

I think we should hardcode the URLs instead of making them localizable. The other way to do it would be to insert the locale code somewhere, but thanks to reed, they're working with our autodetect for locales, so that should be good enough. That might need tricks for the MPL, but we can add links there or something. We should have sane links there, though, so rather not expose them to l10n.

aboutRights.dtd should bet a localization note that the a, b, c parts are composed together with the b part being the link title. A note that the content is shown in about:rights might be good, even if it feels obvious now, it probably helps later on.

Please add a localization note for rights.webservices-* to ask for consistency with security.dtd in the preference pane, and that you're reusing the entities from there for that. Localizers might choose to not do that if grammar wants that, but should then pay attention in future changes.

aboutRights.dtd should have a license header, AFAICT.

Comment 31

[Justin Dolske [:Dolske]]

(In reply to comment #30)
> L10n comments [...]

Ok, I've made these changes. Waiting on comments from gavin, and then I'll update the patch.

> aboutRights.dtd should have a license header, AFAICT.

It seems that most other DTD and properties files don't have one.

Comment 32

[Axel Hecht [:Pike]]

Yeah, our story on the license headers is somewhat fuzzy. I think I recall a comment from Gerv along the lines of "they should have, but let's not bloat our code right now by running across all files that don't". For new files, I'd go for it, in particular looking at all the other comments we have in that file in the end, thanks for those :-)

Comment 33

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

Comment on attachment 345408 [details] [diff] [review]
Patch v.3

>diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js

>+pref("browser.EULA.version", 3);

Why's this needed?

>diff --git a/browser/base/Makefile.in b/browser/base/Makefile.in

>+ifneq (,$(BUILD_OFFICIAL)$(MOZILLA_OFFICIAL))
>+DEFINES += -DOFFICIAL_BUILD=1
>+endif

We really need to centralize this define and make it available across the whole tree, and perhaps have it be triggered by --enable-official-branding instead of relying on an envvar at build time. Different bug, though.

>diff --git a/browser/base/content/aboutRights.xhtml b/browser/base/content/aboutRights.xhtml

>+    <li>&rights.webservices-term1;</li>
>+    <li>&rights.webservices-term2;</li>
>+    <li>&rights.webservices-term3;</li>
>+    <li><strong>&rights.webservices-term4;</strong></li>
>+    <li><strong>&rights.webservices-term5;</strong></li>
>+    <li>&rights.webservices-term6;</li>
>+    <li>&rights.webservices-term7;</li>

Perhaps it's worth noting the styling difference in an l10n note.

>diff --git a/browser/components/nsBrowserGlue.js b/browser/components/nsBrowserGlue.js

>   _onBrowserStartup: function()

>+    if(this._shouldShowRights()) {
>+      // XXX bug 460334. Sessionstore hasn't actually restored tabs yet

That bug's fixed, so this is hopefully not an issue now?

>+  _shouldShowRights : function () {

I agree with Johnathan that shouldShow is kinda silly (makes this method harder to read IMO).

>+  _showRightsNotification : function () {
>+    // Stick the notification onto the selected tab of the active browser window.
>+    var win = this.getMostRecentBrowserWindow();
>+    var gBrowser = win.gBrowser; // for closure in notification bar callback
>+    var tab = gBrowser.selectedBrowser;

Now that's just confusing - name it "browser" instead?

>+    var notifyBox = win.getNotificationBox(tab.contentWindow)

If you can access gBrowser, why not just call gBrowser.getNotificationBox(), which defaults to the notification box for the active tab? (nit: missing trailing semi-colon)

>+    // XXX maybe this should just be a fixed string from the brand bundle?

Don't think so, given that we want this localized and independent of branding.

>+#ifdef XP_UNIX
>+#ifndef XP_MACOSX

Bug 450576 suggests that we should define this on Mac as well.

>diff --git a/browser/components/nsIBrowserGlue.idl b/browser/components/nsIBrowserGlue.idl

>+  /**
>+   * Gets the most recent window that's a browser (but not a popup)
>+   */
>+  nsIDOMWindow getMostRecentBrowserWindow();

Can you have nsBrowserContentHandler use this code and remove its getMostRecentBrowserWindow?

>diff --git a/browser/locales/en-US/chrome/browser/aboutRights.dtd b/browser/locales/en-US/chrome/browser/aboutRights.dtd

This could use some extra l10n notes, but Axel's covered that.

>diff --git a/browser/locales/en-US/chrome/browser/aboutRights.properties b/browser/locales/en-US/chrome/browser/aboutRights.properties

>+notifyText = %S is free and open software from the non-profit Mozilla Foundation.

Hmm, the "from the Mozilla Foundation" part is indeed only true for "official builds", so I see why you wanted it tied to branding. Should we have two strings here, one more generic, and determine which to use with an OFFICIAL_BUILD ifdef?

Comment 34

[Kev Needham [:kev]]

There's also a wording change we need in the unbranded builds, per discussion
with Harvey, as follows:

Bullet point three in the "About Your Rights" should be changed to: 

"If this product incorporates web services, any applicable service terms for
the service(s) should be linked to a Web Site Services section." with a link to
the Web Site Services section.

The Web Site Services section should be changed to:

"An overview of the web site services the product incorporates, along with
instructions on how to disable them, if applicable, should be included here."

The first bullet point of the Web Site Services text should remain as-is. Let
me know if you have any questions.

Comment 35

[Justin Dolske [:Dolske]]

Attachment: Patch v.4

Updated with review nits and revised text.

(In reply to comment #33)
> >+pref("browser.EULA.version", 3);
> 
> Why's this needed?

Still accessed by EULA.js, which I'll remove in bug 462254. 

> >+ifneq (,$(BUILD_OFFICIAL)$(MOZILLA_OFFICIAL))
> >+DEFINES += -DOFFICIAL_BUILD=1
> >+endif
> 
> We really need to [...] Different bug, though.

Filed bug 462477.

> >+#ifdef XP_UNIX
> >+#ifndef XP_MACOSX
> 
> Bug 450576 suggests that we should define this on Mac as well.

That or bug 462222 can fix this.

> >+  nsIDOMWindow getMostRecentBrowserWindow();
> 
> Can you have nsBrowserContentHandler use this code and remove its
> getMostRecentBrowserWindow?

Filed bug 462478.

> >+notifyText = %S is free and open software from the non-profit Mozilla Foundation.
> 
> Hmm, the "from the Mozilla Foundation" part is indeed only true for "official
> builds", so I see why you wanted it tied to branding. Should we have two
> strings here, one more generic, and determine which to use with an
> OFFICIAL_BUILD ifdef?

Not sure what to do about this. I suppose it could be interpreted as just meaning the *source* is from MoFo, and anyone actually shipping a product based on the code would presumably change the string?


(In reply to comment #34)
> There's also a wording change we need in the unbranded builds, per discussion
> with Harvey, as follows:

Done.

> "If this product incorporates web services, any applicable service terms for
> the service(s) should be linked to a Web Site Services section."

Nit: "... *the* Web Site Services section"

Comment 36

[Justin Dolske [:Dolske]]

Pushed changeset a131999fa900.

Follow up will continue in other bugs noted above. In particular, bug 462254 will be removing the existing EULA stuff. (Until that's gone, Windows and OS X users won't see the about:rights notification after installing.)

Comment 37

[Axel Hecht [:Pike]]

Found a typo, "the link test" should be "text". We obviously do too much talking about tests and not enough about text ;-)

Other than that, thanks for getting this as good as legalese can get.

Tony, can you create a litmus test for l10n, too? (Duh, now I had "text" there instead of "test")

Comment 38

[Justin Dolske [:Dolske]]

(In reply to comment #37)
> Found a typo, "the link test" should be "text".

Fixed.

Comment 39

[Marcia Knous [:marcia]]

I verified the infobar on Linux using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b2pre) Gecko/20081031 Minefield/3.1b2pre. Per comment 36 will have to wait for that bug fix before final verification on Windows and Linux.

Regarding Comment 34, is that wording supposed to be included in the nightly builds? Bullet point three does not seem to match what I see listed in Comment 34.

Comment 40

[Justin Dolske [:Dolske]]

Oh, crud. Branded Firefox builds should have the correct (branded) text, and people's own development builds should have the correct (unbranded) text, but it looks like MoCo's nightly builds are built in a way that triggers the branded text. :(

I filed bug 462598 to fix this.

Comment 41

[Marcia Knous [:marcia]]

verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081106 Minefield/3.1b2pre, Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081106 Minefield/3.1b2pre, as well as on Windows Vista and Mac Tiger. In Comment 39 I verified on Linux.

Comment 42

[Marcia Knous [:marcia]]

https://litmus.mozilla.org/show_test.cgi?id=7399 added to Litmus BFT and FFT.

Comment 43

[Tony Chung [:tchung]]

(In reply to comment #37)
> Tony, can you create a litmus test for l10n, too? (Duh, now I had "text" there
> instead of "test")

I cloned the one marcia just created into the 3.1 l10n testgroup.
https://litmus.mozilla.org/show_test.cgi?id=7400

Comment 44

[Samuel Sidler (old account; do not CC)]

Dolske: fyi, this bug blocks 1.9.0.5, which freezes in one week on November 17. When you have an appropriate 1.9.0 branch patch, please request approval1.9.0.5 on it.

Comment 45

[Justin Dolske [:Dolske]]

Attachment: Patch v.4 (for branch)

This is the trunk patch v.4, with a few changes for branch:

1. Added the smart _prefs getter from bug 460546, but didn't change existing code to use it.
2. Didn't expose _getMostRecentBrowserWindow() as an interface in nsBrowserGlue.idl to avoid branch interface changes. Instead it's just used as a private method within nsBrowserGlue.
3. Added a wrapper DIV in aboutRights.xhtml because about.css has changed slightly (looks the same as trunk with this change)
4. Included spelling nit from comment 37, as trunk did.

Comment 46

[Justin Dolske [:Dolske]]

Comment on attachment 348110 [details] [diff] [review]
Patch v.4 (for branch)

Requesting approval for branch... Risk is scoped here, the patch mostly adds new stuff to implement about:rights, and only modifies the existing code dealing with deciding when to display the EULA (with just this patch, Windows and OS X users shouldn't see any change).

Comment 47

[Daniel Veditz [:dveditz]]

Comment on attachment 348110 [details] [diff] [review]
Patch v.4 (for branch)

Approved for 1.9.0.5, a=dveditz for release-drivers

Comment 48

[Justin Dolske [:Dolske]]

Checked in on branch:

Checking in browser/app/profile/firefox.js;
  new revision: 1.340; previous revision: 1.339
Checking in browser/base/Makefile.in;
  new revision: 1.24; previous revision: 1.23
Checking in browser/base/jar.mn;
  new revision: 1.126; previous revision: 1.125
Checking in browser/base/content/aboutRights.xhtml;
  initial revision: 1.1
Checking in browser/components/Makefile.in;
  new revision: 1.63; previous revision: 1.62
Checking in browser/components/aboutRights.js;
  initial revision: 1.1
Checking in browser/components/nsBrowserGlue.js;
  new revision: 1.99; previous revision: 1.98
Checking in browser/installer/unix/packages-static;
  new revision: 1.163; previous revision: 1.162
Checking in browser/installer/windows/packages-static;
  new revision: 1.164; previous revision: 1.163
Checking in browser/locales/jar.mn;
  new revision: 1.79; previous revision: 1.78
Checking in browser/locales/en-US/chrome/browser/aboutRights.dtd;
  initial revision: 1.1
Checking in browser/locales/en-US/chrome/browser/aboutRights.properties;
  initial revision: 1.1
Checking in other-licenses/branding/firefox/pref/firefox-branding.js;
  new revision: 1.6; previous revision: 1.5

Comment 49

[Vlado Valastiak [:wladow]]

Comment on attachment 348110 [details] [diff] [review]
Patch v.4 (for branch)

>+<!ENTITY rights.webservices-c " section, and unchecking the options for &quot;&blockAttackSites.label;&quot; and &quot;&blockWebForgeries.label;&quot;">
These entities are m-c stuff and are not present in 1.9.0

Comment 50

[Axel Hecht [:Pike]]

Thanks, wladow. Unmarking fixed1.9.0.5, this is going to break in official builds only.

Comment 51

[Justin Dolske [:Dolske]]

Oops. :( 1.9.1's "blockAttackSites.label" and "blockWebForgeries.label" are the new entity names for 1.9.0's "tellMaybeAttackSite.label" and "tellMaybeForgery.label". I'll check in a fix for this.

Comment 52

[Justin Dolske [:Dolske]]

Attachment: Branch entity name fix v.1

Corrects the entity used within the string that refers to the preferences dialog. rs=mconnor

I didn't rev the "rights.webservices-c" entity name to minimize changes from trunk, and it wasn't clear if other locales had even started translating yet. If those are mistaken assumptions, we can bump the entity name too.

Comment 53

[Justin Dolske [:Dolske]]

Checked in, readding fixed1905 keyword.

Checking in browser/locales/en-US/chrome/browser/aboutRights.dtd;
  new revision: 1.2; previous revision: 1.1

Comment 54

[Axel Hecht [:Pike]]

I initially thought we should just keep the keys, but I reconsidered.

There are still folks in the wild working on 3.0.x code for l10n, and the only locale (pl) that backported on their own so far, haven't caught this.

I suggest to just go for rights.werbservices-c.1.9. As I need to post-process the files anyway, adding this change won't make my life any worse, and it's going to wallpaper us again falling for this later on.

Comment 55

[Justin Dolske [:Dolske]]

Attachment: Branch entity name fix v.2

Update the entity name used, per comment 54.

Checking in browser/base/content/aboutRights.xhtml;
  new revision: 1.2; previous revision: 1.1
Checking in browser/locales/en-US/chrome/browser/aboutRights.dtd;
  new revision: 1.3; previous revision: 1.2

Comment 56

[Al Billings [:abillings - ex-MoCo]]

Verified that this is working in 1.9.0.5 (at least for English) with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5pre) Gecko/2008120105 GranParadiso/3.0.5pre. The first run bar shows up and about:rights shows the text.