Closed Bug 299554 Opened 19 years ago Closed 19 years ago

remove screenshots from cookies.xhtml, recommend "until I close Firefox" and do some cleanup

Categories

(Firefox Graveyard :: Help Documentation, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox1.5

People

(Reporter: steffen.wilberg, Assigned: steffen.wilberg)

Details

Attachments

(1 file, 3 obsolete files)

Bug 299344 removed the screenshots from the Options and the Using the Download
Manager docs. I think we should nuke them in the Cookies doc as well.

"Ask me every time" is a pretty poor recommendation. "Until I close Firefox" is
much better.

"unless I have removed cookies set by that site" is not documented yet.

The "privacy panel"s and "Cookies tab" need some <em>s.
Attached patch patch v0.9 (obsolete) — Splinter Review
This is pretty much done, but I want to read through it again before I request
review.
Attached patch patch v1.0 (obsolete) — Splinter Review
I guess this is not perfect yet, but it should be good enough for review
purposes.

I got rid of the "Cookie Manager" terminology. There is no such manager.
There's an exceptions window and a view cookies window.

The Allow for Session functionality was largely missing.

I removed all screenshots but the "ask dialog" one, since you can't open this
dialog as easily as the exceptions and view cookies windows.
Attachment #188177 - Attachment is obsolete: true
Attachment #188248 - Flags: review?(jwalden+fxhelp)
Comment on attachment 188248 [details] [diff] [review]
patch v1.0

> <h2 id="setting_up_cookie_rules">Setting Up Cookie Rules</h2>
> 
>-<p>By default &brandShortName; accepts all cookies. If you want to gain more control over
>-what cookies are stored, follow these instructions:</p>
>+<p>By default &brandShortName; accepts all cookies. A website might ask
>+  &brandShortName; to store cookies for several years in order to recognize you
>+  even in the far future. If this doesn't suit you, you can tell &brandShortName;
>+  to delete all cookies when you close &brandShortName;:</p>

Remove the middle sentence, and expand the first sentence with, ", including
cookies which would allow a site to recognize you effectively forever."

>+  <li>Click the <em>Privacy</em> panel and then click on the <em>Cookies</em>
>+    tab.</li>

Insert a comma after "panel" here, please.

>+  <li>From the <em>Keep Cookies</em> combo box, select the <em>until I close
>+    &brandShortName;</em> &pref.singular;.</li>

Move "From [...] box" to the end and readjust punctuation/capitalization
accordingly.  This ordering feels a little more natural to me.

>+<p>If you want to grant sites you trust the ability to store cookies permanently,
>+  which is useful for sites you want to e.g. logged in automatically,
>+  click <a href="#cookies_exceptions"><em>Exceptions</em></a>, enter the site,
>+  and click <em>Allow</em>.</p>

Change ", which is useful for sites you want to e.g. logged in automatically"
to " (e.g., to allow you to log in automatically to a site)".  Also, add "
address" after "enter the site", please.

>+    cookies are disabled. If you want to grant certain sites the ability to
>+    store cookies, open the <a href="#cookies_exceptions">Cookies Exceptions
>+    Window</a> by clicking <em>Exceptions</em>, enter the site, and click

Once again, add " address" after "enter the site".

>   <ul>
>     <li><strong>until they expire</strong>: If this &pref.singular; is selected,
>+      cookies will be removed when they hits their expiration date.</li>

All cookies don't have the same expiration date, so this can be interpreted to
be misleading.	How about:

If this &pref.singular; is selected, each cookie will be removed when it
reaches its expiration date.

>+  <dd>If you don't want this cookie to be stored, click <em>Deny</em>. Use this
>+    if you don't trust the site or suspect that it's compromising your privacy.</dd>
>+  <dt>Use my choice for all cookies from this site</dt>
>+  <dd>Select this checkbox before clicking any of the buttons if you want
>+    &brandShortName; to remember your decision and not ask again. The site will
>+    be added to the <a href="#cookies_exceptions">Cookie Exceptions Window</a>,
>+    where you can revert your choice.</dd>

Rhetorically, why would the user want to revert his choice?  Maybe adding
"should you wish to do so later" at the end would solve the problem.

>+<p>Access this window by clicking the <em>Exceptions</em> button in cookies
>+  &pref.plural;. Here you can make exceptions from your general cookies
>+  &pref.plural; for specific sites. For example, if you have disabled cookies in
>+  general, you can still allow cookies for certain sites. Or if you have chosen
>+  to keep cookies until you exit &brandShortName; in general, you can still
>+  allow cookies for certain sites to be kept permanently. In the Status column,
>+  you can see if a site is blocked, allowed, or allowed for session.</p>

Your example uses are too long.  How about this sentence instead of "For
example [...] kept permanently.":

Using exceptions, you can allow all cookies, reject all cookies, or allow
cookies which will be kept until you exit &brandShortName;, regardless of your
other cookie settings.

I don't really, really like this suggestion, but I think it's at least
marginally better.

>+  cookies from the site, click <em>Allow</em> to allow cookies from the site,
>+  or click <em>Allow for Session</em> to allow cookies from the site, but have
>+  them deleted when you exit &brandShortName;.</p>

I'm pretty sure you don't need the comma after "to allow cookies from the
site".

>+<p>To remove a site from this list, select it and click <em>Remove Site</em>. To
>+  clear the list completely, click <em>Remove All Sites</em>. This will clear
>+  the exceptions list, so your general cookies &pref.plural; apply.</p>

I don't think you need the comma after "exceptions list", either.

Anyway, assuming these changes meet with your approval, r=me.
Attachment #188248 - Flags: review?(jwalden+fxhelp)
Attachment #188248 - Flags: review+
Attachment #188248 - Flags: approval-aviary1.1a2?
I'm following all comments, except this one:
Instead of "Using exceptions, you can allow all cookies, reject all cookies, or
allow cookies which will be kept until you exit &brandShortName;...".
This sounds like Firefox only accepts session cookies, which would be wrong.
It doesn't matter if the site wants to store a permanent cookie or a session
cookie. If you specify "Allow for Session", Firefox accepts all cookies, but
degrades them to session cookies, so they will be deleted when you exit Firefox
even if the site wanted to store a permanent cookie.
I suggest "Using exceptions, you can allow all cookies, reject all cookies, or
allow all cookies but have them deleted when you exit &brandShortName;...".
Attached patch patch v1.1 (obsolete) — Splinter Review
I also fixed the toc entry from "Other Settings" to "Cookie Settings", because
I've renamed it in the document from "Other Settings" to "All the Settings".

Assuming r=jeff still holds.
Attachment #188248 - Attachment is obsolete: true
Attachment #188405 - Flags: review+
Attachment #188405 - Flags: approval-aviary1.1a2?
Attachment #188248 - Flags: approval-aviary1.1a2?
Comment on attachment 188405 [details] [diff] [review]
patch v1.1

Maybe I should diff *before* applying the new patch.
Attachment #188405 - Flags: review+
Attachment #188405 - Flags: approval-aviary1.1a2?
Attached patch patch v1.1Splinter Review
Attachment #188405 - Attachment is obsolete: true
Attachment #188409 - Flags: review+
Attachment #188409 - Flags: approval-aviary1.1a2?
Comment on attachment 188409 [details] [diff] [review]
patch v1.1

A few things which are fixable on checkin:

> <p><img src="&images.baseURL;/cookie_accept.png"
>   alt="" width="286" height="87"/></p>

You forgot to remove this.

>+    be added to the <a href="#cookies_exceptions">Cookie Exceptions Window</a>,
>+    where you can revert your choice, should you wish to do so later.</dd>

You don't need to insert the comma after "choice".

>+  &brandShortName;, regardless of your other cookie settings. In the Status
>+  column, you can see if a site is blocked, allowed, or allowed for session.</p>

I should have noticed the first time through, but the site's not what's being
blocked/allowed/session-allowed -- it's the site's cookies.  Can you change
"site is" to "site's cookies are" to fix this?	Also, "allowed for session"
isn't as understandable as "allowed but deleted when you exit &brandShortName;"
(in the style of other references to session-only cookie behavior, although I'm
uncertain whether the phrasing's possibly too terse), so please change that
too.

In any case tho, the r+ does indeed still hold.
> > <p><img src="&images.baseURL;/cookie_accept.png"
> >   alt="" width="286" height="87"/></p>
> 
> You forgot to remove this.
I left that in intentionally, see comment 2. But on second thought I guess this
can be removed as well along with the other screenshots in this document.

> >+    be added to the <a href="#cookies_exceptions">Cookie Exceptions Window</a>,
> >+    where you can revert your choice, should you wish to do so later.</dd>
> 
> You don't need to insert the comma after "choice".
ok.

> >+  &brandShortName;, regardless of your other cookie settings. In the Status
> >+  column, you can see if a site is blocked, allowed, or allowed for session.</p>
> 
> I should have noticed the first time through, but the site's not what's being
> blocked/allowed/session-allowed -- it's the site's cookies.  Can you change
> "site is" to "site's cookies are" to fix this?
Sure.

> Also, "allowed for session"
> isn't as understandable as "allowed but deleted when you exit &brandShortName;"
> (in the style of other references to session-only cookie behavior, although I'm
> uncertain whether the phrasing's possibly too terse), so please change that
> too.
That sentence doesn't need to repeat the options (Allow/Allow for Session/Block)
. All it adds to the sentence before is that the status will be displayed in the
status column. So how about this:
"In the Status column, you can see how a site's cookies will be treated."

Of course we could remove that sentence completely.
(In reply to comment #9)
> I left that in intentionally, see comment 2.

Oops, didn't notice that.

> But on second thought I guess this can be removed as well along with the other
> screenshots in this document.

Since I was just requesting what I thought you were intending to do, feel free
to ignore me if you want.  :-)  Of course, if you really think it should be
removed then by all means go ahead with it.

> Of course we could remove that sentence completely.

That's actually probably best.  We go on to explain things better later, so
there isn't much of a reason to keep it.
Attachment #188409 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
Checked in, with both the screenshot and that sentence removed. Most users won't
ever see the Cookie Accept dialog, and it's not that hard to understand the doc
without a screenshot.

Checking in mozilla/browser/locales/en-US/chrome/help/cookies.xhtml;
/cvsroot/mozilla/browser/locales/en-US/chrome/help/cookies.xhtml,v  <-- 
cookies.xhtml
new revision: 1.12; previous revision: 1.11
done
Checking in mozilla/browser/locales/en-US/chrome/help/firebird-glossary.rdf;
/cvsroot/mozilla/browser/locales/en-US/chrome/help/firebird-glossary.rdf,v  <--
 firebird-glossary.rdf
new revision: 1.2; previous revision: 1.1
done
Checking in mozilla/browser/locales/en-US/chrome/help/firebird-index1.rdf;
/cvsroot/mozilla/browser/locales/en-US/chrome/help/firebird-index1.rdf,v  <-- 
firebird-index1.rdf
new revision: 1.23; previous revision: 1.22
done
Checking in mozilla/browser/locales/en-US/chrome/help/firebird-toc.rdf;
/cvsroot/mozilla/browser/locales/en-US/chrome/help/firebird-toc.rdf,v  <-- 
firebird-toc.rdf
new revision: 1.29; previous revision: 1.28
done
Checking in mozilla/browser/locales/en-US/chrome/help/glossary.xhtml;
/cvsroot/mozilla/browser/locales/en-US/chrome/help/glossary.xhtml,v  <-- 
glossary.xhtml
new revision: 1.2; previous revision: 1.1
done
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox1.1
Nuking the images:

Removing cookie_accept.png;
/cvsroot/mozilla-org/html/projects/firefox/help/1.1/cookie_accept.png,v  <-- 
cookie_accept.png
new revision: delete; previous revision: 1.1
done
Removing cookie_ask.png;
/cvsroot/mozilla-org/html/projects/firefox/help/1.1/cookie_ask.png,v  <-- 
cookie_ask.png
new revision: delete; previous revision: 1.1
done
Removing cookie_list.png;
/cvsroot/mozilla-org/html/projects/firefox/help/1.1/cookie_list.png,v  <-- 
cookie_list.png
new revision: delete; previous revision: 1.1
done
Removing cookie_manager.png;
/cvsroot/mozilla-org/html/projects/firefox/help/1.1/cookie_manager.png,v  <-- 
cookie_manager.png
new revision: delete; previous revision: 1.1
done
Removing help-buttons.png;
/cvsroot/mozilla-org/html/projects/firefox/help/1.1/help-buttons.png,v  <-- 
help-buttons.png
new revision: delete; previous revision: 1.1
done
Removing firefox-toolbar.png;
/cvsroot/mozilla-org/html/projects/firefox/help/1.1/firefox-toolbar.png,v  <-- 
firefox-toolbar.png
new revision: delete; previous revision: 1.1
done
Removing print.png;
/cvsroot/mozilla-org/html/projects/firefox/help/1.1/print.png,v  <--  print.png
new revision: delete; previous revision: 1.1
done
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: