250543 - Web Features: rename buttons; disable buttons accordingly to checkboxes

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect, P3)

Description

[PikeUK]

In Tools->Web Features there is a checkbox that says "Allow websites to install
software", directly to the right of this is a button labelled "Exceptions".
However, the word "Exceptions" doesn't make sense in the context of "Allow
websites...". Software installation has to be explicitly allowed for each site
therefore the entries in that box are not exceptions they are additions.

Or am I just being too picky?

Comment 1

[Steffen Wilberg]

I agree. Note that the title of the subdialog for popup and install "exceptions"
is "Allowed Sites". That is because they only have an allow button, no block button.
In contrast to these, the images and cookie (in privacy-cookies) exceptions have
the "block" button.

So the buttons for popup and install should be labelled "Allowed Sites...", and
"Exceptions" should be changed to "Exceptions...".

In addition to that, the popup and image buttons should be disabled if the
checkbox for their feature is deselected. If you're not loading images or allow
installs at all, there's no sense in specifying sites to block. Taking and
requesting blocking due to localization impact.

By the way, modifying the install checkbox has no effect to the pref.

Comment 2

[Steffen Wilberg]

Attachment: patch

Comment 3

[Steffen Wilberg]

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

Some comments:

  var gExceptionsParams = {
    install: { blockVisible: false, allowVisible: true, prefilledHost: "",
permissionType: "install" },
    popup:   { blockVisible: false, allowVisible: true, prefilledHost: "",
permissionType: "popup"   },
>-  image:   { blockVisible: true,  allowVisible: true, prefilledHost: "", permissionType: "image"   },
>+  image:   { blockVisible: true,  allowVisible: true, prefilledHost: "", permissionType: "image"   }
> };
fixes a js strict warning.

> function showExceptions(aEvent)
>@@ -101,10 +119,9 @@
>     existingWindow.focus();
>   }
>   else {
>-    const kURL = "chrome://browser/content/cookieviewer/CookieExceptions.xul?permission=";
>+    const kURL = "chrome://browser/content/cookieviewer/CookieExceptions.xul";
>     var params = gExceptionsParams[aEvent.target.getAttribute("permissiontype")];
>-    window.openDialog(kURL + params.type,
>-                      "_blank", "chrome,modal,resizable=yes", params);
>+    window.openDialog(kURL, "_blank", "chrome,modal,resizable=yes", params);
params.type is undefined. It should've been params.permissionType. But that
isn't needed.
CookieExceptions.js queries window.arguments[0].permissionType, and that works
fine because params is the first argument handed over to the CookieExceptions
dialog, and params contains permissionType.

Comment 4

[PikeUK]

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

Comment 5

[Mike Connor [:mconnor]]

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

Comment 6

[Mike Connor [:mconnor]]

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

Comment 7

[Steffen Wilberg]

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

Comment 8

[Asa Dotzler [:asa]]

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

Please don't request approval until you've received necessary reviews. Thanks.

Comment 9

[Jesse Ruderman]

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

Comment 10

[chris hofmann]

let's get this reviewed and fixed.

Comment 11

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

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

r+a=[e-mail]

Steffen, do you have CVS access yet?

Comment 12

[Mike Connor [:mconnor]]

Steffen, the ellipsis is not required, and is actually wrong per GNOME HIG, if
the action does not _require_ further input of the user.  i.e. File->Save As
requires further interaction, but Edit->Preferences does not (you can click OK
without making any changes).

"Use an ellipsis (...) at the end of the label if the action requires further
input from the user before it can be carried out. For example, Save As... or
Find.... Do not add an ellipsis to commands like Properties, Preferences, or
Settings, as these open windows that do not require further input."

Comment 13

[Michael Lefevre]

I thought that was case for the MS guidelines also, and it appears it is. Google
turns up

"If a command requires additional information to complete its intended action,
and you use a dialog box to supply that information, follow the command with an
ellipsis (...). ... However, not every command that results in the display of a
window should include an ellipsis.
...
Do not include ellipses for commands such as Macros, Help Topics, and Fonts. The
dialog boxes that display these collections may support additional actions, but
the primary intended action of the command is to display the collection."
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnwue/html/ch14d.asp

Also, to reply to Ben's comment, judging from tinderbox showing his checkins,
Steffan does indeed have CVS access now.

Comment 14

[Steffen Wilberg]

Thanks for digging through the docs for me. I'll check this in (yes, myself)
later today without adding the ellipses.

But how about the "Advanced..." button in Web Features? The buttons in General?
Tools->"Options..."? "Manage Bookmarks..."?

Comment 15

[Steffen Wilberg]

Checked into the branch 08/10/2004 15:08, without adding ellipses.
Leaving open for trunk checkin, which depends on bug 241705.

Comment 16

[Steffen Wilberg]

This has been fixed on trunk as well by the branch landing.

Comment 17

[Mike Connor [:mconnor]]

sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs,
filter on "beltznerLovesGoats" to get rid of this mass change