350422 - move google anti-phishing privacy policy out of source

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Tony Chang (Google)]

Before a user can opt into "ask Google" mode for phishing protection, the user is asked to opt into a privacy policy.  The text for this is kept in ./browser/locales/en-US/chrome/browser/safebrowsing/eula.dtd.  This is a bad place for it because it takes a lot of effort to coordinate between localization teams and becomes subject to tri-license requirements.

The proposal is to only include the English text and in other locals, have a link to http://www.mozilla.com/legal/eula/ which would contain a links to anti-phishing  privacy policies on Google's servers.

So, 3 things need to happen: add a link in the source code to http://www.mozilla.com/legal/eula/, add the privacy policy on Google's servers and update the Mozilla web content to point to the privacy policy.

Comment 1

[Brian Rakowski]

It seems like a bad idea to show all non-english speakers an english message explaining the implications of opting in to enhanced mode. Can't the localizers use the same approach that they do to localize the interface?  Admittedly, I don't know anything about the Moz localization process. 

Comment 2

[Tony Chang (Google)]

(In reply to comment #1)
> It seems like a bad idea to show all non-english speakers an english message
> explaining the implications of opting in to enhanced mode. Can't the localizers
> use the same approach that they do to localize the interface?  Admittedly, I
> don't know anything about the Moz localization process. 

You mean have Moz localizers translate the Google privacy policy?  I thought we wanted to be translated by Google localizers.

It's a big headache for Google to translate and hand off to Mozilla (word choice differences, not knowing who's translating what files, not sure what to do for languages that Google doesn't normally translate into, etc).

Comment 3

[Brian Rakowski]

I see. I forgot that this was being translated by Google. Would it be acceptable to serve the popup as an iframe in the xul popup? If there is a concern about not being able to reach the server when the user is offline, we could fall back to English text that lives in the tree.

Comment 4

[Sherman Dickman]

I think this would be an interesting approach.  Since the privacy policy is online, and could conceivably change after a user has accepted, perhaps some language needs to be implemented that (a) informs the user that the privacy policy could change, and (b) that they are encouraged to check in at X URL for the most up-to-date privacy policy.

Comment 5

[Mike Connor [:mconnor]]

We already have the appropriate support for remote privacy policies, so we need a URL format to use that we can set per-locale for the correct privacy policy.

Comment 6

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

And preferably a format that we don't need to localize, kthx! :)

Comment 7

[Tony Chang (Google)]

Attachment: v1: use remote privacy policy, fallback on local

The eula display order is now:
- try remote (possibly localized) eula
- try local backup eula
- display error message

There was a suggestion to provide an actual link to the remote eula (while not add any strings).  Any idea on where I should put that?

Comment 8

[Axel Hecht [:Pike]]

Comment on attachment 235993 [details] [diff] [review]
v1: use remote privacy policy, fallback on local

>Index: browser/app/profile/firefox.js
>===================================================================

<...>

>+pref("browser.safebrowsing.provider.0.privacy.fallbackurl", "chrome://browser/content/preferences/phishEULA.xhtml");
> 

AFAICT, you didn't use the .fallbackurl pref but hardcoded the string down the line.

Comment 9

[Tony Chang (Google)]

(In reply to comment #8)
> (From update of attachment 235993 [details] [diff] [review] [edit])
> >Index: browser/app/profile/firefox.js
> >===================================================================
> >+pref("browser.safebrowsing.provider.0.privacy.fallbackurl", "chrome://browser/content/preferences/phishEULA.xhtml");
> > 
> 
> AFAICT, you didn't use the .fallbackurl pref but hardcoded the string down the
> line.
> 

The hardcoded string (chrome://browser/content/preferences/fallbackEULA.xhtml) is the error message that displays if both .url and .fallbackurl fail to load.

Comment 10

[Axel Hecht [:Pike]]

(In reply to comment #9)

Ah, I didn't notice the subtle difference between phishEULA.xhtml and fallbackEULA.xhtml. Got that now, sorry for the confusion.
(Taking the feedom to add needs-review to the status whiteboard)

Comment 11

[Jeff Walden (gone starting June 8)]

Comment on attachment 235993 [details] [diff] [review]
v1: use remote privacy policy, fallback on local

>+    var privacyURL = formatter.formatURLPref("browser.safebrowsing.provider."
>+                                             + providerNum
>+                                             + ".privacy.url",
>+                                             null);
>+    var fallbackURL = formatter.formatURLPref("browser.safebrowsing.provider."
>+                                              + providerNum
>+                                              + ".privacy.fallbackurl",
>+                                              null);

Nit: prevailing style dictates that the "+" go at the end of the line.


>     /**
>      * True if we tried loading the first URL and encountered a failure.
>      */
>-    _loadFailed: false,
>+    _providerLoadFailed: false,
>+    _providerFallbackLoadFailed: false,

Without knowing exactly how the load process happens, the try-primary, try-fallback, try-fallback-fallback series of steps is not immediately obvious to me from the code.  Change the comment above here to give a brief run-through of how we progress through the various failure cases, please?


>+        if (!(aRequest.status & 0x80000000))

Since I originally wrote this, I have been enlightened about the existence of Components.isSuccessCode -- please switch this to that.

Since we're now loading web content in the frame, it might be a good idea to have dveditz give this a sanity-check to be safe; he may want various docshell-level functionalities disabled here.

Removing the DTD from the build *will* affect localizations, because the comparison script detects extra files and entities -- can you run this removal by Axel, please?

Anyway, with the above nits addressed and the above people contacted, this looks okay to me.

Comment 12

[Jeff Walden (gone starting June 8)]

dveditz: see the end of the above comment.

Comment 13

[Axel Hecht [:Pike]]

The removal of that dtd file is intentional, and I think that I will just mass-cover that change on the 1.8 branch once this lands. Not sure about trunk yet, maybe after the branch landing.

Comment 14

[Tony Chang (Google)]

Attachment: v2: incorporates feedback from jwalden

Comment 15

[Mike Connor [:mconnor]]

Comment on attachment 236121 [details] [diff] [review]
v2: incorporates feedback from jwalden

r=me, once we have a real remote URL scheme.  We have less than a week, so please get this URL ASAP.  Otherwise, some smarty will register todogetprivacypolicypath.com and we're screwed ;)

at the least, if we're landing this before the URL is ready, ensure that the .com is replaced with .thisdoesntwork or something similar.

Brian, can you help with getting the URL nailed down here?

Comment 16

[Tony Chang (Google)]

(In reply to comment #15)
> (From update of attachment 236121 [details] [diff] [review] [edit])
> r=me, once we have a real remote URL scheme.  We have less than a week, so
> please get this URL ASAP.  Otherwise, some smarty will register
> todogetprivacypolicypath.com and we're screwed ;)

I should know the URL tomorrow.  I'll update it before I submit.

Comment 17

[Tony Chang (Google)]

Attachment: v3: with real url and http status check

Updated pref to include actual url (not live yet) and http status check for fallback.

Comment 18

[Tony Chang (Google)]

checked in on trunk

Comment 19

[Axel Hecht [:Pike]]

http://www.google.com/tools/firefox/firefox_privacy.html?hl=de and friends still 404s.
When are we going to land this on the branch? Is the [needs-review] status whiteboard still appropriate?

Comment 20

[Tony Chang (Google)]

(In reply to comment #19)
> http://www.google.com/tools/firefox/firefox_privacy.html?hl=de and friends
> still 404s.

The file will be pushed on Tuesday the 12th.

Comment 21

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

Comment on attachment 237254 [details] [diff] [review]
v3: with real url and http status check

a=beltzner on behalf of 181drivers

Comment 22

[Tony Chang (Google)]

on branch

Comment 23

[Axel Hecht [:Pike]]

Note, I just jumped in and CVS removed the l10n files from the MOZILLA_1_8_BRANCH to keep the l10n impact of this landing as small as possible.