359107 - suspected forgery Google TOS refusal renders other choices un-reachable (can't change anti-phishing providers without agreeing to a EULA)

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect)

Description

[Richard Lynch]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0

Install 2.0
Open Tools->Options-Security
In the suspected forgery section, switch to the second radio button:
Check by asking [Google] about each site I visit
Refuse Google's terms of service (cancel) popup window.

Presumably there are other non-Google options in that popup menu.

One cannot reach them without accepting Google's terms of service.

That doesn't seem right...


Reproducible: Always

Steps to Reproduce:
see Details.
Actual Results:  
Forcing me to agree to Google's TOS in order to see my other choices is just plain wrong, imho, no matter how much I like Google.


Expected Results:  
I expected to be able to select choices other than Google before clicking the radio button.

Or you could make a non-valid default in the popup, and not let me continue without choosing one and agreeing to somebody's TOS.

Comment 1

[Zach Lipton [:zach]]

Actually, Google is the only option in that popup menu. If other vendors offered a compatible service, you could install them as your anti-phishing provider of choice and select them instead of Google. Since there are no other choices right now, the Google TOS comes up right away, because you should obviously agree to the TOS before any data is sent to the provider. 

Comment 2

[Tony Chang (Google)]

Actually, I think this is valid.  As in, the drop down list is populated from values in preferences.  So you could install another phishing provider, but can't get to it until you agree to the Google privacy policy.

Of course, when installing another phishing provider, the provider could be smart enough to change the drop down pref for you, but I'm not sure we should count on that.

And this isn't a major issue right now since there are no other providers . . .

Comment 3

[Richard Lynch]

I certainly agree it's a non-issue in terms of functionality if Google is the only service provider so far.

Seems like a UI issue still, and if it won't work right when there IS a second choice, well, then it turns into a real issue RSN.

Obviously it can go on the back burner compared to just about any other (real) issue you guys have flooding in.

I'm not going to be so rude as to change the status back on my own, but I don't think it should go off the radar if it's gonna come back to haunt you as soon as another provider steps up. [shrug]

Maybe "new" and "lowest possible priority" would be best?

Comment 4

[Phil Ringnalda (:philor)]

See also bug 340740 comment 1, where I failed to get anyone to listen to exactly this.

Comment 5

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

*** Bug 359343 has been marked as a duplicate of this bug. ***

Comment 6

[Phil Ringnalda (:philor)]

So, I can see exactly how to fix this, by making the <menulist> enabled, and treating a change in it (once there's something to change it to) trigger switching radio buttons and popping the EULA if it's changed to a not-agreed provider, but I'm not touching it until it has UI Czar mandated UI described.

Comment 7

[Daniel Veditz [:dveditz]]

Not blocking 1.8.1.1 as long as there isn't another provider and no motion on a patch. Good thing to have for the branch, though.

Comment 8

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Patch v1

Comment 9

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

(In reply to comment #6)
> So, I can see exactly how to fix this, by making the <menulist> enabled, and
> treating a change in it (once there's something to change it to) trigger
> switching radio buttons and popping the EULA if it's changed to a not-agreed
> provider, but I'm not touching it until it has UI Czar mandated UI described.

That would work, though it would be a little unusual to have a UI element activated on an unselected option. I'm not sure if there's another sensible way to do this with instant-apply, though.

(Of course, when we get back into the world of multiple providers, then the local list is downloaded from a different source as well, isn't it? That gets us back to the original design for this preference which had the provider listed in the primary checkbox line, and then the radio buttons were options about the "level" of protection.)

Comment 10

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Branch patch

Uses the branch-safe approach from comment 6. Also includes a fix for bug 355780.

Comment 11

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

Comment on attachment 247150 [details] [diff] [review]
Branch patch

uir=me assuming this is the screenshot you showed me over email

Comment 12

[Daniel Veditz [:dveditz]]

Do we need a different reviewer? Mano? Neil?

Comment 13

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Branch patch v2

Cleaned up a few things in the original patch.

Comment 14

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Attachment: Branch patch v3

Fix a few things spotted by Gavin.

Comment 15

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

Comment on attachment 251854 [details] [diff] [review]
Branch patch v3

r=me, but I'm a bit curious as to why this is a 1.8.1.x branch blocker. Neither this nor bug 355780 really seem all that serious, especially given that we only have one provider at the moment. I'm not really sure that the benefits of taking this patch on the branch outweigh the risks in this case.

Comment 16

[Jay Patel [:jay]]

Comment on attachment 251854 [details] [diff] [review]
Branch patch v3

Approved for 1.8 branch, a=jay for drivers.

We think this is the right thing to do to be prepared for the future (if we ever work with other partners for phishing protection)... but if others have good reasons why we want this, please share you thoughts.  Thanks.

Comment 17

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

mozilla/browser/components/preferences/security.js 	1.9
mozilla/browser/components/preferences/security.xul 	1.14

Comment 18

[Jay Patel [:jay]]

Gavin:  Just a reminder to land this on the 1.8 branch when you get a chance. Thanks!

Comment 19

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

mozilla/browser/components/preferences/security.js 	1.3.2.7
mozilla/browser/components/preferences/security.xul 	1.5.2.8

Comment 20

[Carsten Book [:Tomcat]]

verified fixed for 1.8.1.2. Tested with Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.2pre) Gecko/20070126 BonEcho/2.0.0.2pre ID:2007012603 and Linux Fedora FC6