253944 - enablePrivilege dialog uses "Yes" and "No" as button labels

Status: VERIFIED FIXED

(Core :: Security, defect, P3)

Description

[Jesse Ruderman]

The enablePrivilege dialog uses "Yes" and "No" as button labels.  Dialogs
shouldn't use "Yes" and "No" as button labels, espeically security dialogs.

Comment 1

[Jesse Ruderman]

Attachment: a simple page that calls enablePrivilege

Comment 2

[Stephen Donner [:stephend] Not actively reading bugmail]

Error: uncaught exception: A script from "http://bugzilla.mozilla.org" was
denied UniversalXPConnect privileges.

Comment 3

[Jesse Ruderman]

stdonner: You have to enable codebase principles in about:config, save the
script to your hard drive, or get an attacker to sign the script.

Comment 4

[Ben Goodger (use ben at mozilla dot org for email)]

patch by the end of the week or this misses the boat. 

Comment 5

[chris hofmann]

blake, any update?

Comment 6

[Steffen Wilberg]

Attachment: use OK/Cancel

The question can be answered by OK/Cancel as well:
A script from "%S" has requested %S privileges. You should grant these
privileges only if you are comfortable downloading and executing a program from
this source. Do you wish to allow these privileges?

Comment 7

[Jesse Ruderman]

Err, ok/cancel is even worse than yes/no.  Maybe grant/cancel?

Comment 8

[Steffen Wilberg]

Comment on attachment 156223 [details] [diff] [review]
use OK/Cancel

Brian, this is really simple, yet a 1.0PR blocker.

Comment 9

[Steffen Wilberg]

Attachment: use Grant/Cancel

Grant/Cancel this time.
This patch is for the aviary branch.
caps.properties used to live in /caps/src/caps.properties.

Comment 10

[Steffen Wilberg]

Comment on attachment 156227 [details] [diff] [review]
use Grant/Cancel

Grant/Cancel is better indeed.

Comment 11

[Steffen Wilberg]

My cvs account is limited to browser/ and toolkit/. Can somebody check this in
for me, please? See comment 9 for trunk checkin.

Comment 12

[Daniel Veditz [:dveditz]]

I've more or less incorporated this into the fix for bug 253942, except I went
with Allow/Deny because the dialog wording changed somewhat, and I reused the
existing Yes/No caps strings so we won't have blank buttons in locales that
don't catch on to the upgrade.

Hm, maybe blank strings are better (easier to tell it's broken)? Except I'm
afraid this is a dialog that doesn't get a lot of testing or use so it might not
be caught either way.

I see this is all reviewed and approved so I'll merge around it if you land first.

Comment 13

[Steffen Wilberg]

Blank buttons? No. If there's no "Grant" string, there's no dialog, and the
request is denied:
    rv = sStrBundle->GetStringFromName(NS_LITERAL_STRING("Grant").get(),
                                       getter_Copies(grant));
    if (NS_FAILED(rv))
        return PR_FALSE;

But I agree that showing Yes and No buttons is better than not showing the dialog.

Comment 14

[Daniel Veditz [:dveditz]]

Fixed on trunk plus aviary and 1.7 branches

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

    rv = sStrBundle->GetStringFromName(NS_LITERAL_STRING("Grant").get(),
                                       getter_Copies(grant));
    if (NS_FAILED(rv))
        return PR_FALSE;


fwiw as I recall, this function happily succeeds and returns an empty string if
the requested string is not found

Comment 16

[Christian :Biesinger (don't email me, ping me on IRC)]

then again, the code looks like it does return a failure in that case, so I
think I'm wrong.

Comment 17

[Daniel Veditz [:dveditz]]

This patch is not exactly what was checked in... in the final patch in bug
253942 the existing Yes/No strings were reused (with new text) so that any
lagging translations will still have something that makes sense.

Comment 18

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Dialog shot

Comment 19

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED on trunk Seamonkey using 2004-11-12-04 on Windows XP.