Cookie confirmation dialog could use better wording.

RESOLVED FIXED

Status

()

Core
Networking: Cookies
RESOLVED FIXED
16 years ago
15 years ago

People

(Reporter: Christian Reis, Assigned: Michiel van Leeuwen (email: mvl+moz@))

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(8 attachments, 3 obsolete attachments)

(Reporter)

Description

16 years ago
This is a carry-over from bug 23508.

The current cookie dialog uses the following wording:

   /|\ The site X wants to set/modify a/an existing cookie. Do you want to allow it?
    [ ] Remember this decision

    (( Yes )) ( No )

This wording is long (even for an ascii mockup) and prompts the user with a
Yes/No question. I provide a small mockup to suggest new wording:

    /|\  The site .foo.bar wants to set a/modify an existing cookie.  
   / * \ [ ] Use this choice for all cookies from this site.
     
  ( Show details )                   (  Allow  ) ((  Deny  ))

Rationale:

- I think the question "Do you want to allow it?" isn't necessary, since
Allow/Deny already matches the intention in "wants". 
- The change would make the dialog less wide and easier to scan, which is a good
thing for any dialog.

Removing the question and offering a choice (i.e. no Yes/No buttons) also makes
the dialog slighly less intrusive, but that's not the main issue here. Michiel
has already coded the change for 23508 so I'm assigning to him in hope he wants
to take this as soon as bug 23508 is checked in. If not, reassign to nobody, as
always.

Comment 1

16 years ago
also, the word 'contents' is a bit more descriptive than 'information' when 
describing the value of the cookie. to be consistent, same should be changed in 
cookie manager dialog.
(Reporter)

Updated

16 years ago
Depends on: 23508
(Assignee)

Comment 2

16 years ago
See also bug 142277
(Assignee)

Comment 3

16 years ago
Attachment 63889 [details] in bug 23508 suggest tu use vertical oriented buttons. That
seems a good idea to me. 
It also suggest not to use a checkbox, but use 'accept all' and 'decline all'
buttons. Seems good too.

(Should we use Decline or Deny? I am no native english speaker. Is there any
difference?)
Status: NEW → ASSIGNED
(Assignee)

Comment 4

16 years ago
Created attachment 100591 [details]
too wide dialog

It can happen that the dialog is much too wide. See screenshot. That should be
preventend with new wording. Some newlines won't hurt.
(Assignee)

Comment 5

16 years ago
Created attachment 101435 [details]
Suggestion for dialog

Attaching a suggestion. I changed some wording, but alsa some of the layout. If
that should nog happen in this bug, let me know. We can open another bug.

Comment 6

16 years ago
If your concern is about the dialog being too wide, you can always add
line-breaks to the text so that it appears on several narrow lines instead of
one wide one.
(Assignee)

Comment 7

16 years ago
That is not my only concern.
I think accept/reject is more clear then yes/no. That way, you only have to
reaad the first line to decide which button to click.
That is also why I made the first line bold. That is all you need to know. The
rest is extra information.

Also, it seems to mee that the current wording is not consitent when you have
mote than 2 cookies from one site. "The site foo.bar wants to set a second
cookie. " and "The site foo.bar wants permission to set another cookie".

And the twisty just looks better.
(Reporter)

Comment 8

16 years ago
The problem with the twisty is that it's not as obvious as the button, but since
it was in the original spec, it's acceptable as long as clicking on the "About
this label" cookie also triggers it.

I would really recommend removing "Do you want to remove it?" -- accept and
reject  already complement the "wants to modify" statement (and wants already
denotes intention).

I also think "Remember decision" is being unclear at best, and false at worse.
What the checkbox does is "Use this choice for all cookies from this site",
though you can use "Remember this decision..." if you strongly disagree with that.

Comment 9

16 years ago
> I think accept/reject is more clear then yes/no. That way, you only have to
> reaad the first line to decide which button to click.

I agree but I think that 'Allow' and 'Deny' would be better than 'Accept' 
and 'Reject'. The reason for this is that 'Accept' sounds like physically 
accepting an item (and 'Reject' like physically rejecting something). This is 
fine for setting cookies but not so good for modifying cookies. If it was "The 
site www.test.localdomain has offered to modify an existing cookie.", 'Accept' 
(accepting the offer) and 'Reject' (rejecting the offer) would be fine. But the 
site isn't offering, it's asking your (the user's) permission. In light of 
this, I think 'Allow' and 'Deny' would be better.

> That is also why I made the first line bold. That is all you need to know. The
> rest is extra information.

Nice touch. :-)

> The problem with the twisty is that it's not as obvious as the button, but
> since it was in the original spec, it's acceptable as long as clicking
> on the "About this label" cookie also triggers it.

I understand that twisties ('disclosure triangles') like this are more common 
on Macs than on Windows and Linux. Presumably Mac users will have no problem 
with the twisty but Windows and Linux users may do. Therefore I think a button 
would be better.

> I would really recommend removing "Do you want to remove it?" -- accept and
> reject  already complement the "wants to modify" statement (and wants already
> denotes intention).

I agree. Less text is good (as long as it doesn't reduce the meaning). If that 
happens though, the remaining text should not be bold.

> I also think "Remember decision" is being unclear at best, and false at worse.
> What the checkbox does is "Use this choice for all cookies from this site",
> though you can use "Remember this decision..." if you strongly disagree with 
> that.

Just to be clear, am I right in thinking that checking the box and:

a) Clicking 'Accept' allows the site to set and modify cookies at will?
b) Clicking 'Reject' does not allow the site to set any new cookies nor modify 
any existing ones?

If so, then perhaps 'Allow All' and 'Deny All' buttons would be better. Or 
maybe the checkbox should use wording closer to IE's ("Apply my decision to all 
cookies from this Web site").
(Reporter)

Comment 10

16 years ago
> Just to be clear, am I right in thinking that checking the box and:
>
> a) Clicking 'Accept' allows the site to set and modify cookies at will?
> b) Clicking 'Reject' does not allow the site to set any new cookies nor modify 
> any existing ones?

mpt's original spec called for Accept All/Reject All buttons with Accept/Reject
buttons in the disclosed part. I think it *might* be the best solution, but I'm
not sure if 4 buttons (5 if you have a disclosure button) is going to be confusing.

- Apply my decision to all cookies from this Web site
- Use this decision for all cookies from this site
- Use my choice for all cookies from this site

I think the last and shortest option is the best for a checkbox. "Web site" and
"site" are interchangeable, as "decision" and "choice". I think "use" is better
than apply.

I also suggest Allow/Deny, as the mockup I opened the bug with indicates.
(Reporter)

Comment 11

16 years ago
> Just to be clear, am I right in thinking that checking the box and:

Sorry. Yes. :-)
(Assignee)

Comment 12

16 years ago
I agree about Allow and Deny.

> If so, then perhaps 'Allow All' and 'Deny All' buttons would be better. Or 
> maybe the checkbox should use wording closer to IE's ("Apply my decision to all 
> cookies from this Web site").

My problem with 'Allow All' is it is not clear to me (as user) if it means
'Allow All cookies form this site' or 'Allow All cookies from all sites'. A
checkbox can have a much longer label then a button, so no problem in that respect.
"Use my choice for all cookies from this site" sounds good to me.

In the screenshot, I used vertical buttons. That looks better to me, and is what
is in mpt's spec. I that allright? It is not used much in the rest of mozilla.
(Assignee)

Comment 13

16 years ago
Created attachment 102853 [details]
updated suggestion

My updated suggestion. It has:
- allow / deny buttons
- a more window-ish more-info button instead of a twisty.
- a short header with the problem.
- a little more info, with an actual question. without a question, what do you
answer?
- a change in the checkbox text

I don't have a patch yet, as there is still have a problem. The width of the
more/less info button changes, and so the width of the dialog changes too. That
is ugly. But in the meantime, comments are welcome.

Comment 14

16 years ago
Comment on attachment 102853 [details]
updated suggestion

on windows readonly fields do not look like editable textareas. only disabled
textareas should appear the way you see the fields in your picture.

Note: This is a skin issue, so for win classic they shouldn't look like
disabled editable textareas, but if some other skin wanted to make it look the
way you showed it in your picture, that would be a decision for the skin.
(Assignee)

Comment 15

16 years ago
Created attachment 104321 [details] [diff] [review]
change wording and layout

I am uplocaing my opatch anyway. I have not found a solution for my problem.
But it only shows when pressing the moreinfo button. I think most people just
leave it  in one state. But ideas for fixing it are welcome.

Timeless: it are readonly textboxes. So linux and windows differ. If windows
displays them like disabled editable textareas, it is a bug in windows classic.

Updated

16 years ago
Component: User Interface Design → Cookies
(Reporter)

Comment 16

15 years ago
*** Bug 142277 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 17

15 years ago
I've discussed this a bit with Michiel and I think he's going to work out
another (final?) layout so we can get this reviewed.
(Assignee)

Comment 18

15 years ago
Created attachment 113961 [details]
new screenshot

A new suggestion.

Horizontal buttons to solve previous problems. Right aligned labels.

Comment 19

15 years ago
Yes, this is a nice, compact layout and the wording is clear, much better than
my "solution" from bug 142277.
(Assignee)

Comment 20

15 years ago
Adding suresh, for comments on the screenshots.
(Reporter)

Comment 21

15 years ago
In comment 2, moshe proposed using "Content" instead of "Information" and I
think it makes a lot of sense. mvl?

My only other reservation is towards having the text with the number of cookies.
Does anybody actually care how many cookies they have from that site? My mom
sure doesn't, and I never have either. I'd rather have less text on a dialog
than more.

Anyway, this is my only comment. I think this dialog is *MUCH* nicer than the
original one, so let's hope we can get some review and checkin.
(Reporter)

Comment 22

15 years ago
*** Bug 163380 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 23

15 years ago
> In comment 2, moshe proposed using "Content" instead of "Information" and I
> think it makes a lot of sense. mvl?
Yes, that makes sense. I will change it.

> My only other reservation is towards having the text with the number of 
> cookies. Does anybody actually care how many cookies they have from that site? 
> My mom sure doesn't, and I never have either. I'd rather have less text on a 
> dialog than more.
Your mom doesn't use the dialog at all :)
The text is there because it always was. I don't use it. I think we can remove it.
Do you want the difference between setting a first and setting another cookie to
go to?

Comment 24

15 years ago
Adding Jatin to cc list.

Jatin, can you review the wordings/strings posted in the screenshot? Thanks!

Comment 25

15 years ago
I would not remove the number of existing cookies. It does not take much space,
and it may be useful to some people. If you really do not like it then at least
put it into the part that appears with the details.
[And if you completely remove it, then why would you want the first line in bold
;-) ?]
(Assignee)

Comment 26

15 years ago
Just putting things in the UI (or leaving them) because they may be usefull to
somebody isn't going to work. That way, the UI will be unusable because of all
things that might be usefull to someone.
So we need a better reason then maybe usefull for someone. At least /why/ it
might be usefull.

Comment 27

15 years ago
As I said, it does not take much space and it doesn't clutter the UI at all. And
it is really bad to remove a feature that has been there for so long.

Why? Hmm, for me it is sometimes useful, because of the hundreds of sites
wanting to set cookies I forget which cookies I really need and if I see that I
already have cookies from a site I am reminded that I may need it from this one
and after looking at the details I can decide...
(Reporter)

Comment 28

15 years ago
> Just putting things in the UI (or leaving them) because they may be usefull to
> somebody isn't going to work. That way, the UI will be unusable because of all
> things that might be usefull to someone.

Michiel, you're a swell guy. This sums up the point precisely. And I would add
on: "just because you think you can come up with use case for something doesn't
mean it's a *real* use case." We're not even typical users.

Anyway, I would *recommend* all of this cookie counting were removed. The simple
plain message works fine for all cases, I'm absolutely positive of it, in fact. 

However, this is a delicate point: arguments in Bugzilla get started over
nothing, and I honestly don't want this bug to be held down because people start
complaining about us cleaning up the UI to make it saner. So if people are going
to start screaming (as they did when we removed toolbar grippies) that Mozilla
is going to suck because it doesn't tell you how many cookies you have set in a
prompt dialog that 0.3% of the end-users will see, I would rather we stopped
here, checked in what people have seemed to agree upon, and count ourselves
lucky this time. The lack of UI authority just makes working on it too scary,
we're lucky this bug has but a few people CC:ed on it.

And yes, grippies were turned back on.
(Assignee)

Comment 29

15 years ago
Created attachment 114122 [details] [diff] [review]
Patch v2

The patch that gives the previous screenshot.
Only changed information -> content.
With the number of cookies. We can file a seperate bug on that, to prevent long
discussions here. This part is ready in my opinion.
Attachment #104321 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #114122 - Flags: superreview?(darin)
Attachment #114122 - Flags: review?(suresh)

Comment 30

15 years ago
Comment on attachment 114122 [details] [diff] [review]
Patch v2

> +        <image id="info.icon" class="spaced" src="chrome://global/skin/icons/alert-exclam.gif"/>

do *not* hard code skin paths into xul.

dbaron doesn't particularly like the idea of '.' in an id, given how it'd
interact with css i'd have to agree.

i believe you ignored my comment about readonly fields v. selectable labels,
but that's not a showstopper.
Attachment #114122 - Flags: superreview?(darin)
Attachment #114122 - Flags: review?(suresh)
Attachment #114122 - Flags: review-
(Assignee)

Comment 31

15 years ago
Created attachment 114254 [details] [diff] [review]
Use class=plain for textboxes

Using class="plain" now for hte textboxes, to remove the border. To be
consistent, also update the cookieviewer.
And changed the id="info.box". btw, we should also fix
http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/commonDialog.xul#26
but that is beyond the scope of this bug.
(Assignee)

Comment 32

15 years ago
Created attachment 114255 [details]
screenshot that goes with previous patch
(Assignee)

Comment 33

15 years ago
Created attachment 114354 [details] [diff] [review]
Fix skin work

I forgot the skin stuff. Done now by using <image id="infoicon" class="spaced
alert-icon"/>
Attachment #114254 - Attachment is obsolete: true

Updated

15 years ago
Attachment #114354 - Flags: review+
(Assignee)

Updated

15 years ago
Attachment #114354 - Flags: superreview?(darin)

Comment 34

15 years ago
Comment on attachment 114354 [details] [diff] [review]
Fix skin work

deferring UI superreview to jag ;-)
Attachment #114354 - Flags: superreview?(darin) → superreview?(jaggernaut)

Comment 35

15 years ago
Comment on attachment 114354 [details] [diff] [review]
Fix skin work

-  dialog.getButton("accept").label = dialog.getAttribute("acceptLabel");
-  dialog.getButton("cancel").label = dialog.getAttribute("cancelLabel");
+  document.getElementById("ok").label = dialog.getAttribute("acceptLabel");
+  document.getElementById("cancel").label =
dialog.getAttribute("cancelLabel");

Why this change?

I'll take another look after jatin comments on the new wording.

Comment 36

15 years ago
Looks good to me except the sentence, "View and Remove Cookies that are
stored..." should be rewritten to say, "View and remove cookies that are stored..."
(Reporter)

Comment 37

15 years ago
One minor detail too: we are using Hostname on the cookie confirmation dialog,
versus Domain in the cookie details dialog. Is this intentional? I thought both
represented domain, but let me check... 

Oh! We currently use Domain: in the confirmation dialog. I don't think it should
be changed to hostname because, after all, it's a domain :-)
(Assignee)

Comment 38

15 years ago
>-  dialog.getButton("accept").label = dialog.getAttribute("acceptLabel");
>-  dialog.getButton("cancel").label = dialog.getAttribute("cancelLabel");
>+  document.getElementById("ok").label = dialog.getAttribute("acceptLabel");
>+  document.getElementById("cancel").label =
>dialog.getAttribute("cancelLabel");
>
>Why this change?

I had to use on overlay to be able to put the button not at the bottom of the
window. But now the getButton() function doesn't work anymore, maybe because the
overlay doesn't use dlgtype to define the buttons. 


>Looks good to me except the sentence, "View and Remove Cookies that are
>stored..." should be rewritten to say, "View and remove cookies that are
>stored..."

I didn't touch that, but can fix it while I am there.


>One minor detail too: we are using Hostname on the cookie confirmation dialog,
>versus Domain in the cookie details dialog. Is this intentional? I thought both
>represented domain, but let me check... 

No, we use both in both places :)
Domain when the cookie is set for the whole domain, so .mozilla.org as domain
atribute. That cookie will be sent to every host in the mozilla.org domain.
Host when a cookie has no domain attribute, which means it wil only be sent to
the host that set it. And in the screenshot, there is one host cookie, and one
domain cookie.

Comment 39

15 years ago
> I had to use on overlay to be able to put the button not at the bottom of 
> the window. But now the getButton() function doesn't work anymore, maybe 
> because the overlay doesn't use dlgtype to define the buttons.

Hmmm... Either dialogOverlay.xul (and its platform versions) should be made to
work together with the dialog binding (add dlgtype, make oncommand handlers work
with dialog binding's command dispatch mechanism), or we need to add support to
the dialog binding to move the buttons around.

For now, sr=jag on your patch.

Updated

15 years ago
Attachment #114354 - Flags: superreview?(jaggernaut) → superreview+

Comment 40

15 years ago
Checking in extensions/cookie/resources/content/cookieAcceptDialog.js;
/cvsroot/mozilla/extensions/cookie/resources/content/cookieAcceptDialog.js,v 
<--  cookieAcceptDialog.js
new revision: 1.5; previous revision: 1.4
done
Checking in extensions/cookie/resources/content/cookieAcceptDialog.xul;
/cvsroot/mozilla/extensions/cookie/resources/content/cookieAcceptDialog.xul,v 
<--  cookieAcceptDialog.xul
new revision: 1.4; previous revision: 1.3
done
Checking in extensions/cookie/resources/locale/en-US/cookieAcceptDialog.dtd;
/cvsroot/mozilla/extensions/cookie/resources/locale/en-US/cookieAcceptDialog.dtd,v
 <--  cookieAcceptDialog.dtd
new revision: 1.2; previous revision: 1.1
done
Checking in extensions/cookie/resources/locale/en-US/cookieAcceptDialog.properties;
/cvsroot/mozilla/extensions/cookie/resources/locale/en-US/cookieAcceptDialog.properties,v
 <--  cookieAcceptDialog.properties
new revision: 1.3; previous revision: 1.2
done
Checking in extensions/wallet/cookieviewer/CookieViewer.dtd;
/cvsroot/mozilla/extensions/wallet/cookieviewer/CookieViewer.dtd,v  <-- 
CookieViewer.dtd
new revision: 1.13; previous revision: 1.12
done
Checking in extensions/wallet/cookieviewer/CookieViewer.xul;
/cvsroot/mozilla/extensions/wallet/cookieviewer/CookieViewer.xul,v  <-- 
CookieViewer.xul
new revision: 1.63; previous revision: 1.62
done

I checked the patch in for mvl, fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
(Reporter)

Comment 41

15 years ago
>>Looks good to me except the sentence, "View and Remove Cookies that are
>>stored..." should be rewritten to say, "View and remove cookies that are
>>stored..."
>
> I didn't touch that, but can fix it while I am there.

Hmm, did you take that into account Robert?

(Assignee)

Comment 42

15 years ago
Created attachment 115243 [details] [diff] [review]
Change last sentence

Ok, I suck. I forgot to update that. Attaching patch that changes it.
(Assignee)

Comment 43

15 years ago
Created attachment 115255 [details] [diff] [review]
Also fix js error

There is an JS error on the console too. This patch fixes that. (And the
centence I forgot)
Attachment #115243 - Attachment is obsolete: true
(Assignee)

Comment 44

15 years ago
Reopening for the js error and missed sentence
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Updated

15 years ago
Attachment #115255 - Flags: superreview?(darin)
Attachment #115255 - Flags: review?(suresh)

Comment 45

15 years ago
Comment on attachment 115255 [details] [diff] [review]
Also fix js error

r-suresh
Attachment #115255 - Flags: superreview?(jaggernaut)
Attachment #115255 - Flags: superreview?(darin)
Attachment #115255 - Flags: review?(suresh)
Attachment #115255 - Flags: review+

Comment 46

15 years ago
Nice use of readonly="true" class="plain"

Updated

15 years ago
Attachment #115255 - Flags: superreview?(jaggernaut) → superreview+
Checking in cookie/resources/content/cookieAcceptDialog.js;
/cvsroot/mozilla/extensions/cookie/resources/content/cookieAcceptDialog.js,v 
<--  cookieAcceptDialog.js
new revision: 1.6; previous revision: 1.5
done
Checking in wallet/cookieviewer/CookieViewer.dtd;
/cvsroot/mozilla/extensions/wallet/cookieviewer/CookieViewer.dtd,v  <-- 
CookieViewer.dtd
new revision: 1.14; previous revision: 1.13
done
Status: REOPENED → RESOLVED
Last Resolved: 15 years ago15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.