Closed Bug 279506 Opened 20 years ago Closed 19 years ago

Document new Options window

Categories

(Firefox Graveyard :: Help Documentation, defect)

Product:
Firefox Graveyard
Component:
Help Documentation
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
Firefox1.5

People

(Reporter: jwalden+fxhelp, Assigned: steffen.wilberg)

References

(
URL
)

Details

Attachments

(2 files, 5 obsolete files)

patch v2.0
86.31 KB, patch
jwalden+fxhelp
: review+
Details | Diff | Splinter Review
what I checked in
89.84 KB, patch
Details | Diff | Splinter Review 
It's so radically different from the previous one that our docs barely even
describe it.  Our screenshots are completely out of date; buttons have changed;
and we have, alternately, tabs and selectable option lists in use to subdivide
prefs.  Basically, someone took the old one, rearranged things so that the
layout actually made sense at the top-level ("Web Features"?), and left us with
a ton of work to do.

This needs to be started ASAP, beginning with what Ben has posted online.  I
estimated 10 days when posting on the wiki, but I have a feeling it just might
take a bit longer than that -- unless, that is, we can get it fixed by starting
now and finishing it before the end of the month (after which point my available
time is reduced pretty dramatically).
I'll try to start working on this really soon (within two-ish days -- I'm close
to either cracking bug 253334 or finding out I'm way off), but if other people
can get something finished first, we'll go with that.  In a few minutes I'm
going to go and bump the URL base string for images, so make sure to edit the
new ones if you start uploading -- we don't want to disturb the images used in 1.0.
Flags: blocking-aviary1.1+
Target Milestone: --- → Firefox1.1
It looks different, a lot has been moved around (like default character encoding
from the languages subdialog to the fonts subdialog), but I guess we can reuse
80-90% of the text. We've even documented the Certificates and Validation stuff
already.

From Ben's blog: "Most notable new features include: Sanitize function,
searchable group-by-host cookies, and a searchable download actions manager (now
with integrated full page plugin support)."

I'd suggest to let the dust settle a bit. Ben said he'll land later this week
(http://www.mozillazine.org/talkback.html?article=5940#10).
Depends on: 274712
I'll start working on this.
Assignee: jwalden+fxhelp → steffen.wilberg
(In reply to comment #3)
> I'll start working on this.

Start on earlier sections, then, and I'll grab some of the later sections, if
you want.  Next week is free time for me, so I was planning on doing it then. 
As another option, I could work on other existing bugs, if you'd prefer.

From a previous stab I made at this, I'd prefer if you'd do it in small bits. 
The RDF changes will be decently expansive, and doing it all at once is sure to
introduce errors that we won't notice until it's too late.
Attached patch first stab at prefs.xhtml (obsolete) — — Splinter Review 
I worked my way through prefs.xhtml. I didn't touch index or toc yet. It's not
100% final, but I'd appreciate comments on it. I'm afraid I was a bit
overzealous in indenting the file, but that makes it a lot easier to read the
source. But I had to move around a lot anyway.

The new Help style is pretty useful, I'm using <h4> in a number of cases.

Do you think we need the screenshots? I like it better without them. I've
disabled them with a display:none css rule for now.

It's still a pretty long document. Maybe we should split it into the 6
sections.
Attached patch patch v1.0 (obsolete) — — Splinter Review 
Phew, the rest was actually not that hard. I don't think the "In this section"
box of prefs.xhtml should be this verbose, but it's pretty useful to check all
the links.

Now it's your turn, Jeff! :-)
Attachment #177747 - Attachment is obsolete: true
Attachment #177928 - Flags: review?(jwalden+fxhelp)
Comment on attachment 177928 [details] [diff] [review]
patch v1.0

Some observations...

>Index: mozilla/browser/components/help/locale/en-US/prefs.xhtml
>===================================================================

<cut>

>-    <h3 class="noMac" id="default_browser">Default Browser</h3>
>+  <h3 class="noMac" id="default_browser">Default Browser</h3>
>     <p class="noMac">You can set &brandShortName; as the default
>       browser by clicking <em>Set Default Browser</em>. This will ensure
>       &brandShortName; is used whenever an application is trying to display a
>       web page or when you open an HTML file.</p>

Can you perhaps fix bug 270353 while you are at it? There is no "Set Default
Browser" button here anymore.

>+    <p><em>Save all files to this folder:</em><br/>
>+      This is the default &pref.singular; in &brandShortName;, and the default
>+      folder is the Desktop. You can specify a different folder where all
>+      downloads will be saved, such as the &quot;My Downloads&quot; folder. You
>+      can browse to a specific folder by selecting <em>Other...</em> from the
>+      drop-down list of available folders. To show the folder, click the
>+      <em>Show Folder</em> button. This will open the folder in the default file
>+      manager.</p>

There is now a "Browse" button that replaces the drop-down and the "Show
Folder" button.

>+  <h3 id="download_manager">Download Manager</h3>
>+    <p><em>Show Download Manager when a download begins</em> and
>+      <em>Close the download manager when all downloads are complete</em><br/>
>+      Select wheter or not you want the download manager to automatically appear
>+      when a download begins, and whether to be closed when all downloads are
>+      complete.</p>

"Select wheter" -> "Select whether"

>+  <h3 id="download_actions">Download Actions</h3>
>+    <p>The Download Actions dialog, which can be opened by clicking the<em>View
>+      &amp; Edit Actions...</em>button, contains file types that you have
>+      downloaded. You can choose what &brandShortName; should do when clicking
>+      on a specific file type. Select the file type you want to modify and click
>+      the <em>Change Action...</em> button. To remove an automatic rule for a
>+      file type, select that file type and click the <em>Remove Action</em>
>+      button.</p>
>+
>+    <p>This will display the Change Action dialog, where you can choose to have
>+      the file type opened by the default application, opened by an user-selected
>+      application, saved to disk, or shown by using an installed plugin. For
>+      example, if you view lots of media files on web pages, you might want to
>+      specify that &brandShortName; always open media files in your media player
>+      instead of asking where you want each media file to be saved.</p>

These two paragraphs seem to say that the "Remove Action" button opens the
"Change Action" dialog. 

>+      <p><em>Manage Security Devices</em><br/>
>+        Security devices can encrypt and decrypt connections and store
>+        certificates and passwords. If you need to use a security device other
>+        than the one in &brandShortName;, click the <em>Manage Security
>+        Devices...</em> button.</p>

This button is now labeled "Security Devices".

>Index: mozilla/browser/components/help/locale/en-US/firebird-toc.rdf
>===================================================================

<cut>

>+        <rdf:li> <rdf:Description ID="prefs-advanced-general" nc:name="General tab" nc:link="prefs.xhtml#advanced_general"/> </rdf:li>
>+        <rdf:li> <rdf:Description ID="prefs-advanced-update" nc:name="Update tab" nc:link="prefs.xhtml#advanced_update"/> </rdf:li>
>+        <rdf:li> <rdf:Description ID="advanced-security" nc:name="Security tab" nc:link="prefs.xhtml#advanced_security"/> </rdf:li>

Is there any good reason to not give this last item the ID
"prefs-advanced-security"?
Comment on attachment 177928 [details] [diff] [review]
patch v1.0

I'll get to this sometime tomorrow.  At a glance, however, the previous
comments seem worthy of some consideration (and probably in most if not all
cases, implementation).
Attachment #177928 - Attachment is obsolete: true
Attachment #177928 - Flags: review?(jwalden+fxhelp)
Blocks: 270353
Attached patch patch v1.1 (obsolete) — — Splinter Review 
Thanks for the very good comments, I've adressed all of them.

I've also removed the "alt" texts from the of screenshots per bug 280587. I
still think we should remove the screenshots however.
Attachment #178040 - Flags: review?(jwalden+fxhelp)
Comment on attachment 178040 [details] [diff] [review]
patch v1.1

>Index: mozilla/browser/components/help/locale/en-US/prefs.xhtml
>===================================================================

>+  <h3 class="noMac" id="default_browser">Default Browser</h3>
>+    <p class="noMac"><em>&brandShortName; should check to see if it is the default
...
>+      an HTML file. Use the <em>Check Now</em> button to to a check right now.</p>

"to to" -> "to do"

>+      <p><em>Use autoscrolling</em><br/>
...
>+        move the mouse up or down. Some people find this annoying, so autoscroll
>+        can be disabled with this &pref.singular;.</p>

Should "autoscroll" be "autoscrolling"?
Blocks: 280587
Attached patch patch v1.1.1 (obsolete) — — Splinter Review 
Comment 10 addressed.
Attachment #178040 - Attachment is obsolete: true
Attachment #178068 - Flags: review?(jwalden+fxhelp)
Attachment #178040 - Flags: review?(jwalden+fxhelp)
Comment on attachment 178068 [details] [diff] [review]
patch v1.1.1

>   <ul>
>     <li><a href="#general_options">General &pref.pluralCaps;</a>
>     <ul>
>-      <li><a href="#fonts_and_colors">Fonts &amp; Colors</a></li>
>-      <li><a href="#languages">Languages</a></li>
>       <li><a href="#connection_settings">Connection Settings</a></li>
>     </ul>
>     </li>
>     <li><a href="#privacy_options">Privacy &pref.pluralCaps;</a></li>
>-    <li><a href="#web_features_options">Web Features &pref.pluralCaps;</a></li>
>-    <li><a href="#downloads_options">Download &pref.pluralCaps;</a></li>
>+    <ul>
>+      <li><a href="#privacy_history">History tab</a></li>
>+      <li><a href="#privacy_saved_forms">Saved Forms tab</a></li>
>+      <li><a href="#privacy_passwords">Passwords tab</a></li>
>+      <li><a href="#privacy_download_history">Download History tab</a></li>
>+      <li><a href="#privacy_cookies">Cookies tab</a></li>
>+      <li><a href="#privacy_cache">Cache tab</a></li>
>+    </ul>
>+    <li><a href="#content_options">Content &pref.pluralCaps;</a></li>
>+    <li><a href="#tabs_options">Tabs &pref.pluralCaps;</a></li>
>+    <li><a href="#downloads_options">Downloads &pref.pluralCaps;</a></li>
>     <li><a href="#advanced_options">Advanced &pref.pluralCaps;</a></li>
>+    <ul>
>+      <li><a href="#advanced_general">General tab</a></li>
>+      <li><a href="#advanced_update">Update tab</a></li>
>+      <li><a href="#advanced_security">Security tab</a></li>
>+    </ul>
>   </ul>

We had multiple levels of content expanded here previously, partially as a
holdover from the last options redesign that made Fonts & Colors part of
General.  Now that options are actually organized in a coherent manner,
however, I'm not sure they're really needed (particularly as adding multiple
levels bloats it up so much now).  Thoughts?

>+  <h3 id="home_page">Home Page</h3>
>+    <p>Here you are able to specify the page (or tab group) that
>+      &brandShortName; will show when you launch it or press the Home button.

I don't particularly like the "Here you are able to" phrasing that exists in
this patch and in current code.  What do you think of changing "are able to" to
"can"?	It still feels a little awkward, but I don't think it's quite so bad
any more.

>+    <p>To specify the home page(s) using a bookmark, click <em>Use
>+      Bookmark....</em> You can even select a whole bookmark folder to be
>+      used!</p>

First and most trivially, we need to move one of the periods inside the <em/>
outside of it.	Second, I don't really like the last sentence here. 
Exclamation marks should be used sparingly in docs (if at all), and it smacks
of, "Yay, let's get excited about this cool feature!!!!"  Here's how I rewrote
it:

	Selecting a whole bookmark folder will load the bookmarks inside it in
tabs on startup.

Thoughts on this suggested change?

>+  <h3 class="noMac" id="default_browser">Default Browser</h3>
>+    <p class="noMac"><em>&brandShortName; should check to see if it is the default
>+      browser when starting</em><br/>
>+      Check this option if you want &brandShortName; to check whether it is the
>+      default browser when starting. This will ensure &brandShortName; is used
>+      whenever an application is trying to display a web page or when you open
>+      an HTML file. Use the <em>Check Now</em> button to do a check right now.</p>

s/whenever an application is trying/whenever an application tries/
s/Use/You can also click/

>     <p>Your organization or internet service provider may offer or require you

s/internet/Internet/

>+  <p>Alternatively you can specify items you want to be cleared when you use the
>+    &accelKey;+&shiftKey;+<kbd>Del</kbd> keyboard shortcut or close
>+    &brandShortName; in the <em>Sanitize Settings...</em> dialog.</p>

As it is now, this reads, "...you can specify items you want to be cleared when
you close Firefox in the Sanitize Settings... dialog."	Firefox can't be closed
in that dialog, obviously, so we need to rephrase this.  :-)

>+  <h3 id="privacy_history">History tab</h3>
>+    <p>Here you can specify for how many days you want the browser to remember
>+      what pages you have visited. The default is 9 days.</p>

Two problems here: first, we refer to Firefox as "the browser".  Second, the
phrasing of "for how many days" is rather ugly; what about:

      Here you can specify how long you want &brandShortName; to remember what
      pages you have visited.

>+  <h3 id="privacy_saved_forms">Saved Forms tab</h3>
>+    <p><em>Save information I enter in forms and the Search bar</em><br/>

"bar" needs to be capitalized here to agree with the text in the UI.  (As a
side note, this review is making me think that switching over to entities
sooner [perhaps even before 1.1] rather than later is becoming a better and
better idea.)

>+    <p id="remove_master_password"><em>Remove Master Password...</em><br/>
>+      Click this button to remove your current master password.</p>

It'd be nice if we could mention that the master password is required in order
to do this.

>+    <p>The Download Manager (accessible from <span class="menuPath">Tools</span>
>+      or by pressing
>+      <span class="noUnix">&accelKey;+<kbd>J</kbd></span><span class="unix">&accelKey;+<kbd>Y</kbd></span>)
>+      stores shortcuts to your recent downloads. The Download &pref.plural; are
>+      available in the <strong>Downloads panel</strong>.</p>

We should change "The Download &pref.plural;" to "Downloads &pref.plural;",
because that section of options is named with the plural form of download.

>+    <p><em>Remove files from the Download Manager:</em><br/>
>+      Choose whether the list of the recently downloaded files is purged <em>upon
>+      successful download</em>, or <em>when &brandShortName; exits</em>, or
>+      whether you want to purge the list <em>manually</em> by clicking the
>+      <em>Clear Download History Now</em> button below, or the <em>Clean Up</em>
>+      button in the Download Manager.</p>

This is one huge sentence, and it needs to be broken up into multiple
sentences.  The logical place to split is probably after the "manually" option,
where we switch from describing the dropdown to describing how to delete
download history.

>+  <h3 id="privacy_cache">Cache tab</h3>
>+    <p>Pages you view are normally stored in a special cache folder for quicker
>+      viewing the next time you visit the same page. Here you are able to
>+      specify the amount of disk space the cache can use.</p>

What do you think of changing the last sentence to:

You can specify the amount of disk space the cache can use here.

>+    <p><em>Default Font</em> and <em>Size</em><br/>
>+      Normally, web pages are displayed in the default font or in a font chosen
>+      by the web pages' authors.</p>

This description seems rather sparse.

>+      <p><em>Allow pages to choose their own colors</em><br/>
>+        By default, &brandShortName; uses the colors specified by the web page
>+        author. Disabling this &pref.singular; will force all sites to use your
>+        default colors instead.</p>

The name for this preference is slightly different from how you've specified
it.

>+    <p><em>Show Download Manager when a download begins</em> and
>+      <em>Close the download manager when all downloads are complete</em><br/>
>+      Select whether or not you want the download manager to automatically appear
>+      when a download begins, and whether to be closed when all downloads are
>+      complete.</p>

The option names sort of smash together when they're displayed like this
because they're so long.  I'm not sure what the best fix is here -- describing
them separately /probably/ would work best.

>+  <h3 id="download_actions">Download Actions</h3>
>+    <p>The Download Actions dialog, which can be opened by clicking the<em>View

I think you need a space there at the end. :-)

>+      downloaded. You can choose what &brandShortName; should do when clicking
>+      on a specific file type. Select the file type you want to modify and click
>+      the <em>Change Action...</em> button.</p>

Let's unify those last two sentences, like so:

      You can choose what &brandShortName; should do when clicking on a
specific
      file type by selecting the file type you want to modify and clicking the
      <em>Change Action...</em> button.

>+    <p>This will display the Change Action dialog, where you can choose to have
>+      the file type opened by the default application, opened by an user-selected
>+      application, saved to disk, or shown by using an installed plugin.

Two things: change "an" to "a" right before "user-selected", and change "by
using" to "with".

>+      <p><em>Begin finding as you begin typing</em><br/>
>+        When this &pref.singular; is enabled, &brandShortName; will find within

This has been renamed to "Begin finding when you begin typing".

>+      <p><em>Use smooth scrolling</em><br/>
>+        While Smooth scrolling is far from being perfect in &brandShortName;, it
>+        can be very useful if you read a lot of long pages. Normally, when you
>+        press the Page Down key, the view jumps directly down one page. With
>+        Smooth Scrolling, it slides down more smoothly, so you are actually able
>+        to see how much it scrolls. This makes it easier to resume reading from
>+        where you were before.</p>

First, let's use consistent capitalization here and go with "smooth scrolling"
throughout the text.  Second, do we really need to trumpet smooth scrolling as
being far from perfect?  Is there anything wrong with eliminating everything
from "While" through the end of the line and replacing it with "Smooth"? 
Third, let's replace "are actually able to" with just "can" -- less wordy and
more clear.  Finally, as a thought, what do you think about adding "Smooth
scrolling may perform slowly on older computers." to the end of the paragraph? 
I think that would address most of the "issues" that make smooth scrolling "far
from perfect" in Firefox, although I'm not really sure whether or not it's
worth adding it.

>+  <h3 id="advanced_update">Update tab</h3>
>+    <p>&brandShortName; can check whether a new version of your installed
>+      extensions and Themes, or of &brandShortName; itself is available.</p>

There's some verb-direct object agreement problems here, and the comma doesn't
split the sentence well.  My suggestion for a fix:

      <p>&brandShortName; can check whether new versions of your installed
	extensions, themes, or &brandShortName; itself are available.</p>

>+    <p>Click the <em>Check Now</em> button to manually perform a check for
>+      updates to &brandShortName; and installed extensions.</p>

How about adding "appropriate" before "<em>Check Now</em>" and changing the
"and" near the end to "or"?

>+        type of encryption instead of what is automatically selected), select
>+        the &pref.singular; "Ask Every Time" and you'll be in complete control
>+        of what certificates you use while browsing.</p>

It's now "Ask me every time".

>+      <p><em>Manage Security Devices</em><br/>
>+        Security devices can encrypt and decrypt connections and store
>+        certificates and passwords. If you need to use a security device other
>+        than the one in &brandShortName;, click the <em>Security Devices</em>
>+        button.</p>

This button is just "Security Devices" now, as you noted in the paragraph text,
but the section itself is still mis-titled.  :-)

>         <rdf:li> <rdf:Description ID="prefs-privacy" nc:name="Privacy" nc:link="prefs.xhtml#privacy_options"/> </rdf:li>
>-        <rdf:li> <rdf:Description ID="prefs-web-features" nc:name="Web Features" nc:link="prefs.xhtml#web_features_options"/> </rdf:li>
>+        <rdf:li> <rdf:Description ID="prefs-content" nc:name="Content" nc:link="prefs.xhtml#content_options"/> </rdf:li>
>+        <rdf:li> <rdf:Description ID="prefs-tabs"    nc:name="Tabs"    nc:link="prefs.xhtml#tabs_options"/> </rdf:li>
>         <rdf:li> <rdf:Description ID="prefs-download" nc:name="Downloads" nc:link="prefs.xhtml#downloads_options"/> </rdf:li>

Could we change "prefs-download" to "prefs-downloads", to better fit with the
panel name?

>       <rdf:Seq>
>-        <rdf:li> <rdf:Description ID="prefs-history" nc:name="History" nc:link="prefs.xhtml#history"/> </rdf:li>
>-        <rdf:li> <rdf:Description ID="prefs-saved-form-information" nc:name="Saved Form Information" nc:link="prefs.xhtml#saved_form_information"/> </rdf:li>
>-        <rdf:li> <rdf:Description ID="prefs-saved-passwords" nc:name="Saved Passwords" nc:link="prefs.xhtml#saved_passwords"/> </rdf:li>
>-        <rdf:li> <rdf:Description ID="prefs-download-manager-history" nc:name="Download Manager History" nc:link="prefs.xhtml#download_manager_history"/> </rdf:li>
>-        <rdf:li> <rdf:Description ID="prefs-cookies" nc:name="Cookies" nc:link="prefs.xhtml#cookies"/> </rdf:li>
>-        <rdf:li> <rdf:Description ID="prefs-cache" nc:name="Cache" nc:link="prefs.xhtml#cache"/> </rdf:li>
>+        <rdf:li> <rdf:Description ID="prefs-history" nc:name="History tab" nc:link="prefs.xhtml#privacy_history"/> </rdf:li>
>+        <rdf:li> <rdf:Description ID="prefs-saved-forms" nc:name="Saved Forms tab" nc:link="prefs.xhtml#privacy_saved_forms"/> </rdf:li>
>+        <rdf:li> <rdf:Description ID="prefs-passwords" nc:name="Passwords tab" nc:link="prefs.xhtml#privacy_passwords"/> </rdf:li>
>+        <rdf:li> <rdf:Description ID="prefs-download-history" nc:name="Download History tab" nc:link="prefs.xhtml#privacy_download_history"/> </rdf:li>
>+        <rdf:li> <rdf:Description ID="prefs-cookies" nc:name="Cookies tab" nc:link="prefs.xhtml#privacy_cookies"/> </rdf:li>
>+        <rdf:li> <rdf:Description ID="prefs-cache" nc:name="Cache tab" nc:link="prefs.xhtml#privacy_cache"/> </rdf:li>
>       </rdf:Seq>

I don't really think the " tab" part needs to be in the names for these tree
items.	(The same goes for in prefs.xhtml, where I sort of skimmed over that
fact mentally.)  Also, I think it would be a good idea to add in an item for
"Sanitize", because otherwise you can't get to it from Privacy prefs in the
tree without making a guess, and it clearly belongs there.

>       <rdf:Seq>
>-        <rdf:li> <rdf:Description ID="prefs-accessibility" nc:name="Accessibility" nc:link="prefs.xhtml#accessibility"/> </rdf:li>
>-        <rdf:li> <rdf:Description ID="prefs-browsing" nc:name="Browsing" nc:link="prefs.xhtml#browsing"/> </rdf:li>
>-        <rdf:li> <rdf:Description ID="prefs-tabbed-browsing" nc:name="Tabbed Browsing" nc:link="prefs.xhtml#tabbed_browsing"/> </rdf:li>
>-        <rdf:li> <rdf:Description ID="prefs-updates" nc:name="Software Update" nc:link="prefs.xhtml#software_update"/> </rdf:li>
>-        <rdf:li> <rdf:Description ID="prefs-security" nc:name="Security" nc:link="prefs.xhtml#security"/> </rdf:li>
>-        <rdf:li> <rdf:Description ID="prefs-certificates" nc:name="Certificates" nc:link="prefs.xhtml#certificates"/> </rdf:li>
>-        <rdf:li> <rdf:Description ID="prefs-validation" nc:name="Validation" nc:link="prefs.xhtml#validation"/> </rdf:li>
>+        <rdf:li> <rdf:Description ID="prefs-advanced-general" nc:name="General tab" nc:link="prefs.xhtml#advanced_general"/> </rdf:li>
>+        <rdf:li> <rdf:Description ID="prefs-advanced-update" nc:name="Update tab" nc:link="prefs.xhtml#advanced_update"/> </rdf:li>
>+        <rdf:li> <rdf:Description ID="prefs-advanced-security" nc:name="Security tab" nc:link="prefs.xhtml#advanced_security"/> </rdf:li>
>       </rdf:Seq>

Similarly, I don't think we need " tab" here either.

>                 <rdf:li><rdf:Description ID="saving:page" nc:name="Web Page" nc:link="using_firebird.xhtml#saving_all_or_part_of_a_page"/></rdf:li>
>-                <rdf:li><rdf:Description ID="saving:form" nc:name="Form" nc:link="prefs.xhtml#saved_form_information"/></rdf:li>
>+                <rdf:li><rdf:Description ID="saving:form" nc:name="Form" nc:link="prefs.xhtml#saved_forms"/></rdf:li>
>                 <rdf:li><rdf:Description ID="saving:passwords" nc:name="Passwords" nc:link="prefs.xhtml#saved_passwords"/></rdf:li>

The nc:link for Form should be "prefs.xhtml#privacy_saved_forms", and for
Passwords it should be "prefs.xhtml#privacy_passwords".

> <h2 id="customizing">Customizing Tabbed Browsing</h2>
> 
> <p>To customize tabbed browsing, select &pref.menuPath; and open the
>-  <em>Advanced</em> panel.  &pref.pluralCaps; for tabbed browsing can be found
>-  under <a href="prefs.xhtml#tabbed_browsing">Tabbed Browsing</a>.</p>
>+  <em>Advanced</em> panel. &pref.pluralCaps; for tabbed browsing can be found
>+  under <a href="prefs.xhtml#tabs_options">Tabbed Browsing</a>.</p>

This stuff's been moved to the Tabs panel now, remember?  :-)

>     <tr>
>+      <td>Sanitize &brandShortName;</td>
>+      <td>&accelKey;+&shiftKey;+<kbd>Del</kbd></td>
>+      <td>&nbsp;</td>
>+      <td>&nbsp;</td>
>+    </tr>

I thought there was some sort of similar shortcut in Opera, so I did a brief
search to see if I could find any references to it on the web.	First result I
ran across was <http://users.accesscomm.ca/gbraun/opera.htm>, which mentioned
(Alt-T,D) as the shortcut to do this.  This is just a regular menu shortcut
according to <http://www.opera.com/support/tutorials/security/shared/>,
however, so I don't really see a reason to accord it any special status here.

> <h2 id="the_cookie_manager">Managing Cookies</h2>
> 
> <p>Use the Cookie Manager to view and remove cookies and manage per-site cookie
>-  &pref.plural;.  It is accessible through the Cookies section in the Privacy tab of
>+  &pref.plural;. It is accessible through the Cookies section in the Privacy tab of
>   &pref.singularCaps;.</p>

s/Cookies section in the Privacy tab/Cookies tab in the Privacy panel/

> <p>To remove a cookie from the list, select it and click <em>Remove Cookie</em>.
>+  Select a site to remove all cookies of that site. To wipe all cookies, click

The instructions on removing all cookies for a site are somewhat misleading,
because you have to select it first and then click the Remove Cookies button.

>+  <em>Remove All Cookies</em>. (This is the same as clicking the <em>Clear</em>
>+  button from the <a href="prefs.xhtml#privacy_options">Privacy panel</a> of
>+  the &pref.pluralCaps; window.)</p>

The parenthesized instructions are out of date and need to be updated.

Other things:

As discussed on IRC, we need to be careful about OK/Cancel references, because
theoretically you shouldn't see OK/Cancel in Linux (and Macs?).  The best bet's
probably just to remove them, because it's too easy to get bitten.  Searching
for "Cancel" is easier than me making a list of the ones I find, so I'm punting
that task.  :-)

There are a bunch of things that need updating in using_firebird.xhtml. 
Plug-ins needs updating.  The "Helper Applications" section has a decent amount
of prefs info that needs updating.  There's a section on setting Firefox as
default browser that needs updating.  There's a Cache section in there that
should be updated, too.

popup.xhtml has some outdated instructions on accessing popup options.

Sanitize needs to be added to firebird-toc.rdf underneath the menu reference.

In prefs.xhtml, the titles for the sections shouldn't have " tab" in them,
similar to how they shouldn't have " tab" in the toc/index file (whichever it
was, I've forgotten).

By the way, when you post a patch responding to these issues, please mention
how you dealt with each of them.  I'm not looking forward to another review of
this magnitude if it can be avoided by only looking at the parts that change.
Attachment #178068 - Flags: review?(jwalden+fxhelp) → review-
(In reply to comment #12)
> We had multiple levels of content expanded here previously, partially as a
> holdover from the last options redesign that made Fonts & Colors part of
> General.  Now that options are actually organized in a coherent manner,
> however, I'm not sure they're really needed (particularly as adding multiple
> levels bloats it up so much now).  Thoughts?
This takes far too much space, and isn't needed because individual sections are
easily accessible via the toc. I've removed all but the main entries.


> >+  <h3 id="home_page">Home Page</h3>
> >+    <p>Here you are able to specify the page (or tab group) that
> >+      &brandShortName; will show when you launch it or press the Home button.
> 
> I don't particularly like the "Here you are able to" phrasing that exists in
> this patch and in current code.  What do you think of changing "are able to" to
> "can"?	It still feels a little awkward, but I don't think it's quite so bad
> any more.
Changed to "can".


> >+    <p>To specify the home page(s) using a bookmark, click <em>Use
> >+      Bookmark....</em> You can even select a whole bookmark folder to be
> >+      used!</p>
> 
> First and most trivially, we need to move one of the periods inside the <em/>
> outside of it.
Fixed.


> Second, I don't really like the last sentence here. 
> Exclamation marks should be used sparingly in docs (if at all), and it smacks
> of, "Yay, let's get excited about this cool feature!!!!"
Agreed. I didn't like it either when I read it but forgot to change it.

> Here's how I rewrote it:
> 
> 	Selecting a whole bookmark folder will load the bookmarks inside it in
> tabs on startup.
I think "on startup" is redundant. How about "Selecting a whole bookmark folder
will load all the bookmarks inside it in tabs."?


> >+  <h3 class="noMac" id="default_browser">Default Browser</h3>
> s/whenever an application is trying/whenever an application tries/
> s/Use/You can also click/
Fixed.

> >     <p>Your organization or internet service provider may offer or require you
> s/internet/Internet/
Fixed.

> >+  <p>Alternatively you can specify items you want to be cleared when you use the
> >+    &accelKey;+&shiftKey;+<kbd>Del</kbd> keyboard shortcut or close
> >+    &brandShortName; in the <em>Sanitize Settings...</em> dialog.</p>
> 
> As it is now, this reads, "...you can specify items you want to be cleared when
> you close Firefox in the Sanitize Settings... dialog."	Firefox can't be closed
> in that dialog, obviously, so we need to rephrase this.  :-)
Heh, that's right. It's also missing the new "Sanitize Firefox..." menu item.
How's this:
  <p>&brandShortName; also allows you to clear certain or all items by pressing
    the &accelKey;+&shiftKey;+<kbd>Del</kbd> keyboard shortcut, or selecting the
    <em>Tools &gt; Sanitize &brandShortName;...</em> menu item, or, if you
    choose so, by closing &brandShortName;. Specify these options in the
    <em>Sanitize Settings...</em> dialog.</p>
(How can I improve the "if you choose so" part?)


> >+  <h3 id="privacy_history">History tab</h3>
> >+    <p>Here you can specify for how many days you want the browser to remember
> >+      what pages you have visited. The default is 9 days.</p>
> 
> Two problems here: first, we refer to Firefox as "the browser".  Second, the
> phrasing of "for how many days" is rather ugly; what about:
> 
>       Here you can specify how long you want &brandShortName; to remember what
>       pages you have visited.
Excellent, fixed.


> >+  <h3 id="privacy_saved_forms">Saved Forms tab</h3>
> >+    <p><em>Save information I enter in forms and the Search Bar</em><br/>
> 
> "bar" needs to be capitalized here to agree with the text in the UI.
Fixed.

> (As a
> side note, this review is making me think that switching over to entities
> sooner [perhaps even before 1.1] rather than later is becoming a better and
> better idea.)
That would help a bit indeed, as long as the entity names don't change :)

> >+    <p id="remove_master_password"><em>Remove Master Password...</em><br/>
> >+      Click this button to remove your current master password.</p>
> 
> It'd be nice if we could mention that the master password is required in order
> to do this.
"You have to enter your current master password to do this."?

> >+      stores shortcuts to your recent downloads. The Download &pref.plural; are
> >+      available in the <strong>Downloads panel</strong>.</p>
> 
> We should change "The Download &pref.plural;" to "Downloads &pref.plural;",
> because that section of options is named with the plural form of download.
Fixed.

> >+    <p><em>Remove files from the Download Manager:</em><br/>
> >+      Choose whether the list of the recently downloaded files is purged
<em>upon
> >+      successful download</em>, or <em>when &brandShortName; exits</em>, or
> >+      whether you want to purge the list <em>manually</em> by clicking the
> >+      <em>Clear Download History Now</em> button below, or the <em>Clean Up</em>
> >+      button in the Download Manager.</p>
> 
> This is one huge sentence, and it needs to be broken up into multiple
> sentences.  The logical place to split is probably after the "manually" option,
> where we switch from describing the dropdown to describing how to delete
> download history.
Right. So after that:
      To purge the list,
      click the <em>Clear Download History Now</em> button below, or the
      <em>Clean Up</em> button in the Download Manager.</p>


> >+  <h3 id="privacy_cache">Cache tab</h3>
> >+    <p>Pages you view are normally stored in a special cache folder for quicker
> >+      viewing the next time you visit the same page. Here you are able to
> >+      specify the amount of disk space the cache can use.</p>
> 
> What do you think of changing the last sentence to:
> 
> You can specify the amount of disk space the cache can use here.
OK.


> >+    <p><em>Default Font</em> and <em>Size</em><br/>
> >+      Normally, web pages are displayed in the default font or in a font chosen
> >+      by the web pages' authors.</p>
> 
> This description seems rather sparse.
Right, how about:
      Web pages are usually displayed in the font and size specified here. The
      web pages' authors can choose different fonts and sizes, if you allow that
      in the Advanced Fonts Dialog.</p>

> >+      <p><em>Allow pages to choose their own colors</em><br/>
> >+        By default, &brandShortName; uses the colors specified by the web page
> >+        author. Disabling this &pref.singular; will force all sites to use your
> >+        default colors instead.</p>
> 
> The name for this preference is slightly different from how you've specified
> it.
Looks like I missed the last half sentence, and in the Advanced Fonts Dialog as
well. Fixed.


> >+    <p><em>Show Download Manager when a download begins</em> and
> >+      <em>Close the download manager when all downloads are complete</em><br/>
> >+      Select whether or not you want the download manager to automatically
appear
> >+      when a download begins, and whether to be closed when all downloads are
> >+      complete.</p>
> 
> The option names sort of smash together when they're displayed like this
> because they're so long.  I'm not sure what the best fix is here -- describing
> them separately /probably/ would work best.
I've separated those, and also capitalized Download Manager like everywhere else.

> >+  <h3 id="download_actions">Download Actions</h3>
> >+    <p>The Download Actions dialog, which can be opened by clicking the<em>View
> 
> I think you need a space there at the end. :-)
Indeed, between "the" and "View", as well as between "Edit Actions..." and
"button". Fixed.

> >+      downloaded. You can choose what &brandShortName; should do when clicking
> >+      on a specific file type. Select the file type you want to modify and click
> >+      the <em>Change Action...</em> button.</p>
> 
> Let's unify those last two sentences, like so:
> 
>       You can choose what &brandShortName; should do when clicking on a
> specific
>       file type by selecting the file type you want to modify and clicking the
>       <em>Change Action...</em> button.
Yeah, that's better.


> >+    <p>This will display the Change Action dialog, where you can choose to have
> >+      the file type opened by the default application, opened by an
user-selected
> >+      application, saved to disk, or shown by using an installed plugin.
> 
> Two things: change "an" to "a" right before "user-selected",
Why? "user-selected" starts with a vowel sound, so I need to use "an" instead of
"a".

> and change "by using" to "with".
Fixed.


> >+      <p><em>Begin finding as you begin typing</em><br/>
> >+        When this &pref.singular; is enabled, &brandShortName; will find within
> 
> This has been renamed to "Begin finding when you begin typing".
Good catch, fixed.

> >+      <p><em>Use smooth scrolling</em><br/>
> >+        While Smooth scrolling is far from being perfect in &brandShortName;, it
> >+        can be very useful if you read a lot of long pages. Normally, when you
> >+        press the Page Down key, the view jumps directly down one page. With
> >+        Smooth Scrolling, it slides down more smoothly, so you are actually able
> >+        to see how much it scrolls. This makes it easier to resume reading from
> >+        where you were before.</p>
> 
> First, let's use consistent capitalization here and go with "smooth scrolling"
> throughout the text.
Fixed.

> Second, do we really need to trumpet smooth scrolling as
> being far from perfect?
The original text was "still somewhat experimental"...

> Is there anything wrong with eliminating everything
> from "While" through the end of the line and replacing it with "Smooth"?
"Smooth scrolling" you mean. You're right, it works good enough, so let's remove it.
 
> Third, let's replace "are actually able to" with just "can" -- less wordy and
> more clear.
Fixed.

> Finally, as a thought, what do you think about adding "Smooth
> scrolling may perform slowly on older computers." to the end of the paragraph? 
> I think that would address most of the "issues" that make smooth scrolling "far
> from perfect" in Firefox, although I'm not really sure whether or not it's
> worth adding it.
Let's skip it. If people don't like smooth scrolling, they can disable it again.

> >+  <h3 id="advanced_update">Update tab</h3>
> >+    <p>&brandShortName; can check whether a new version of your installed
> >+      extensions and Themes, or of &brandShortName; itself is available.</p>
> 
> There's some verb-direct object agreement problems here, and the comma doesn't
> split the sentence well.  My suggestion for a fix:
> 
>       <p>&brandShortName; can check whether new versions of your installed
> 	extensions, themes, or &brandShortName; itself are available.</p>
Fixed.


> >+    <p>Click the <em>Check Now</em> button to manually perform a check for
> >+      updates to &brandShortName; and installed extensions.</p>
> 
> How about adding "appropriate" before "<em>Check Now</em>" and changing the
> "and" near the end to "or"?
Sure. I'd also like to mention themes, how about:
    <p>Click the appropriate <em>Check Now</em> button to manually perform a
      check for updates to &brandShortName;, extensions, or themes.</p>

> >+        type of encryption instead of what is automatically selected), select
> >+        the &pref.singular; "Ask Every Time" and you'll be in complete control
> >+        of what certificates you use while browsing.</p>
> 
> It's now "Ask me every time".
Bah. I also used <em>s instead of quotation marks:
        the &pref.singular; <em>Ask me every time</em> and you'll be in complete
        control of what certificates you use while browsing.</p>



> >+      <p><em>Manage Security Devices</em><br/>
> >+        Security devices can encrypt and decrypt connections and store
> >+        certificates and passwords. If you need to use a security device other
> >+        than the one in &brandShortName;, click the <em>Security Devices</em>
> >+        button.</p>
> 
> This button is just "Security Devices" now, as you noted in the paragraph text,
> but the section itself is still mis-titled.  :-)
Thanks, fixed.


> >         <rdf:li> <rdf:Description ID="prefs-privacy" nc:name="Privacy"
nc:link="prefs.xhtml#privacy_options"/> </rdf:li>
> >-        <rdf:li> <rdf:Description ID="prefs-web-features" nc:name="Web
Features" nc:link="prefs.xhtml#web_features_options"/> </rdf:li>
> >+        <rdf:li> <rdf:Description ID="prefs-content" nc:name="Content"
nc:link="prefs.xhtml#content_options"/> </rdf:li>
> >+        <rdf:li> <rdf:Description ID="prefs-tabs"    nc:name="Tabs"   
nc:link="prefs.xhtml#tabs_options"/> </rdf:li>
> >         <rdf:li> <rdf:Description ID="prefs-download" nc:name="Downloads"
nc:link="prefs.xhtml#downloads_options"/> </rdf:li>
> 
> Could we change "prefs-download" to "prefs-downloads", to better fit with the
> panel name?
OK.

> >       <rdf:Seq>
> >+        <rdf:li> <rdf:Description ID="prefs-history" nc:name="History tab"
nc:link="prefs.xhtml#privacy_history"/> </rdf:li>
> >+        <rdf:li> <rdf:Description ID="prefs-saved-forms" nc:name="Saved
Forms tab" nc:link="prefs.xhtml#privacy_saved_forms"/> </rdf:li>
> >+        <rdf:li> <rdf:Description ID="prefs-passwords" nc:name="Passwords
tab" nc:link="prefs.xhtml#privacy_passwords"/> </rdf:li>
> >+        <rdf:li> <rdf:Description ID="prefs-download-history"
nc:name="Download History tab" nc:link="prefs.xhtml#privacy_download_history"/>
</rdf:li>
> >+        <rdf:li> <rdf:Description ID="prefs-cookies" nc:name="Cookies tab"
nc:link="prefs.xhtml#privacy_cookies"/> </rdf:li>
> >+        <rdf:li> <rdf:Description ID="prefs-cache" nc:name="Cache tab"
nc:link="prefs.xhtml#privacy_cache"/> </rdf:li>
> >       </rdf:Seq>
> 
> I don't really think the " tab" part needs to be in the names for these tree
> items.	(The same goes for in prefs.xhtml, where I sort of skimmed over that
> fact mentally.)
I thought it was clearer with tab. Fixed.

> Also, I think it would be a good idea to add in an item for
> "Sanitize", because otherwise you can't get to it from Privacy prefs in the
> tree without making a guess, and it clearly belongs there.
Fixed.

> >       <rdf:Seq>
> >+        <rdf:li> <rdf:Description ID="prefs-advanced-general"
nc:name="General tab" nc:link="prefs.xhtml#advanced_general"/> </rdf:li>
> >+        <rdf:li> <rdf:Description ID="prefs-advanced-update" nc:name="Update
tab" nc:link="prefs.xhtml#advanced_update"/> </rdf:li>
> >+        <rdf:li> <rdf:Description ID="prefs-advanced-security"
nc:name="Security tab" nc:link="prefs.xhtml#advanced_security"/> </rdf:li>
> >       </rdf:Seq>
> 
> Similarly, I don't think we need " tab" here either.
Fixed.

> >                 <rdf:li><rdf:Description ID="saving:page" nc:name="Web Page"
nc:link="using_firebird.xhtml#saving_all_or_part_of_a_page"/></rdf:li>
> >-                <rdf:li><rdf:Description ID="saving:form" nc:name="Form"
nc:link="prefs.xhtml#saved_form_information"/></rdf:li>
> >+                <rdf:li><rdf:Description ID="saving:form" nc:name="Form"
nc:link="prefs.xhtml#saved_forms"/></rdf:li>
> >                 <rdf:li><rdf:Description ID="saving:passwords"
nc:name="Passwords" nc:link="prefs.xhtml#saved_passwords"/></rdf:li>
> 
> The nc:link for Form should be "prefs.xhtml#privacy_saved_forms", and for
> Passwords it should be "prefs.xhtml#privacy_passwords".
Oops, fixed.


> > <h2 id="customizing">Customizing Tabbed Browsing</h2>
> >+  <em>Advanced</em> panel. &pref.pluralCaps; for tabbed browsing can be found
> >+  under <a href="prefs.xhtml#tabs_options">Tabbed Browsing</a>.</p>
> 
> This stuff's been moved to the Tabs panel now, remember?  :-)
Er, right.

> >     <tr>
> >+      <td>Sanitize &brandShortName;</td>
> >+      <td>&accelKey;+&shiftKey;+<kbd>Del</kbd></td>
> >+      <td>&nbsp;</td>
> >+      <td>&nbsp;</td>
> >+    </tr>
> 
> I thought there was some sort of similar shortcut in Opera, so I did a brief
> search to see if I could find any references to it on the web.	First result I
> ran across was <http://users.accesscomm.ca/gbraun/opera.htm>, which mentioned
> (Alt-T,D) as the shortcut to do this.  This is just a regular menu shortcut
> according to <http://www.opera.com/support/tutorials/security/shared/>,
> however, so I don't really see a reason to accord it any special status here.
I agree, i.e. no Opera shortcut.


> > <h2 id="the_cookie_manager">Managing Cookies</h2>
> > 
> > <p>Use the Cookie Manager to view and remove cookies and manage per-site cookie
> >-  &pref.plural;.  It is accessible through the Cookies section in the
Privacy tab of
> >+  &pref.plural;. It is accessible through the Cookies section in the Privacy
tab of
> >   &pref.singularCaps;.</p>
> 
> s/Cookies section in the Privacy tab/Cookies tab in the Privacy panel/
Fixed.

> > <p>To remove a cookie from the list, select it and click <em>Remove Cookie</em>.
> >+  Select a site to remove all cookies of that site. To wipe all cookies, click
> 
> The instructions on removing all cookies for a site are somewhat misleading,
> because you have to select it first and then click the Remove Cookies button.
OK, how about:
  To remove all cookies of a site, select the site and click <em>Remove
  Cookie</em>.


> >+  <em>Remove All Cookies</em>. (This is the same as clicking the <em>Clear</em>
> >+  button from the <a href="prefs.xhtml#privacy_options">Privacy panel</a> of
> >+  the &pref.pluralCaps; window.)</p>
> 
> The parenthesized instructions are out of date and need to be updated.
Indeed.
  (This is
  the same as clicking the <em>Clear Cookies Now</em> button from the <a href="
  prefs.xhtml#privacy_cookies">Cookie tab</a> of the Privacy panel of the
  &pref.pluralCaps; window.)</p>

> Other things:
> 
> As discussed on IRC, we need to be careful about OK/Cancel references, because
> theoretically you shouldn't see OK/Cancel in Linux (and Macs?).  The best bet's
> probably just to remove them, because it's too easy to get bitten.  Searching
> for "Cancel" is easier than me making a list of the ones I find, so I'm punting
> that task.  :-)
I've removed the wo sentences containing Cancel from prefs.xhtml.

> Sanitize needs to be added to firebird-toc.rdf underneath the menu reference.
Fixed.

> In prefs.xhtml, the titles for the sections shouldn't have " tab" in them,
> similar to how they shouldn't have " tab" in the toc/index file (whichever it
> was, I've forgotten).
Fixed, see above.


> There are a bunch of things that need updating in using_firebird.xhtml. 
> Plug-ins needs updating.  The "Helper Applications" section has a decent amount
> of prefs info that needs updating.  There's a section on setting Firefox as
> default browser that needs updating.  There's a Cache section in there that
> should be updated, too.
> 
> popup.xhtml has some outdated instructions on accessing popup options.
I'll deal with those later.


> By the way, when you post a patch responding to these issues, please mention
> how you dealt with each of them.  I'm not looking forward to another review of
> this magnitude if it can be avoided by only looking at the parts that change.
Here you are :)
Thanks for the thorough review!
Attached patch patch v2.0 — — Splinter Review 
Changes per comment 13, plus this one:
> popup.xhtml has some outdated instructions on accessing popup options.

Please have a look at Privacy-Sanitize Settings. Should we move that down, to
the end of the Privacy section? But it corresponds to the sentence before it,
which describes the "Clear" button.

I'll leave using_firebird.xhtml to a separate patch.
Attachment #178068 - Attachment is obsolete: true
Attachment #178118 - Flags: review?(jwalden+fxhelp)
Oh, can you also fix bug 268818 while you are doing this?
(In reply to comment #15)
> Oh, can you also fix bug 268818 while you are doing this?

sorry for bug spamming but NVM for fixing bug 268818
Comment on attachment 178118 [details] [diff] [review]
patch v2.0

> > Here's how I rewrote it:
> > 
> > 	Selecting a whole bookmark folder will load the bookmarks inside it in
> > tabs on startup.
> I think "on startup" is redundant. How about "Selecting a whole bookmark folder
> will load all the bookmarks inside it in tabs."?

Yup, definitely better.  There's a reason we do reviews.  :-)

> > >+  <p>Alternatively you can specify items you want to be cleared when you use the
> > >+    &accelKey;+&shiftKey;+<kbd>Del</kbd> keyboard shortcut or close
> > >+    &brandShortName; in the <em>Sanitize Settings...</em> dialog.</p>
> > 
> > As it is now, this reads, "...you can specify items you want to be cleared when
> > you close Firefox in the Sanitize Settings... dialog."	Firefox can't be closed
> > in that dialog, obviously, so we need to rephrase this.  :-)
> Heh, that's right. It's also missing the new "Sanitize Firefox..." menu item.
> How's this:
>   <p>&brandShortName; also allows you to clear certain or all items by pressing
>     the &accelKey;+&shiftKey;+<kbd>Del</kbd> keyboard shortcut, or selecting the
>     <em>Tools &gt; Sanitize &brandShortName;...</em> menu item, or, if you
>     choose so, by closing &brandShortName;. Specify these options in the
>     <em>Sanitize Settings...</em> dialog.</p>
> (How can I improve the "if you choose so" part?)

Hmm.  Better, I think, but a tad on the wordy side.  Consider this, and let me
know what you think:

  <p>&brandShortName; also allows you to clear certain or all items
    by pressing &accelKey;+&shiftKey;+<kbd>Del</kbd> keyboard
    or selecting <span class="menuPath">Tools &gt; Sanitize
    &brandShortName;...</span>.  You can also change your
    &pref.singular; to make &brandShortName; clear your data when
    you close &brandShortName;.  &pref.singularCaps; for this are in the
    <em>Sanitize &brandShortName;...</em> dialog.</p>

> > >+    <p id="remove_master_password"><em>Remove Master Password...</em><br/>
> > >+      Click this button to remove your current master password.</p>
> > 
> > It'd be nice if we could mention that the master password is required in order
> > to do this.
> "You have to enter your current master password to do this."?

Let's add "will" before "have" here; I think it fits a little better with the
style of how actions that can be performed within the UI are described.

> > >+    <p><em>Remove files from the Download Manager:</em><br/>
> > >+      Choose whether the list of the recently downloaded files is purged
> <em>upon
> > >+      successful download</em>, or <em>when &brandShortName; exits</em>, or
> > >+      whether you want to purge the list <em>manually</em> by clicking the
> > >+      <em>Clear Download History Now</em> button below, or the <em>Clean Up</em>
> > >+      button in the Download Manager.</p>
> > 
> > This is one huge sentence, and it needs to be broken up into multiple
> > sentences.  The logical place to split is probably after the "manually" option,
> > where we switch from describing the dropdown to describing how to delete
> > download history.
> Right. So after that:
>       To purge the list,
>       click the <em>Clear Download History Now</em> button below, or the
>       <em>Clean Up</em> button in the Download Manager.</p>

The comma after "below" should be removed, because we're mentioning two
separate actions that are described with only minimal modification.

> > >+    <p><em>Default Font</em> and <em>Size</em><br/>
> > >+      Normally, web pages are displayed in the default font or in a font chosen
> > >+      by the web pages' authors.</p>
> > 
> > This description seems rather sparse.
> Right, how about:
>       Web pages are usually displayed in the font and size specified here. The
>       web pages' authors can choose different fonts and sizes, if you allow that
>       in the Advanced Fonts Dialog.</p>

This feels kludgy.  How about:

  Web pages are usuallly displayed in the font and size specified
  here.  However, web pages may override these choices unless you
  specify otherwise in the Advanced Fonts dialog.

> > >+    <p>This will display the Change Action dialog, where you can choose to have
> > >+      the file type opened by the default application, opened by an
> user-selected
> > >+      application, saved to disk, or shown by using an installed plugin.
> > 
> > Two things: change "an" to "a" right before "user-selected",
> Why? "user-selected" starts with a vowel sound, so I need to use "an" instead of
> "a".

As discussed on IRC, make the switch to "a" here.

(quoting from a comment with an older snippet of patch because it's easier to
do)
> > >+      <p><em>Use smooth scrolling</em><br/>
> > >+        While Smooth scrolling is far from being perfect in &brandShortName;, it
> > >+        can be very useful if you read a lot of long pages. Normally, when you
> > >+        press the Page Down key, the view jumps directly down one page. With
> > >+        Smooth Scrolling, it slides down more smoothly, so you are actually able
> > >+        to see how much it scrolls. This makes it easier to resume reading from
> > >+        where you were before.</p>

One other thing I didn't catch here before: the Page Down reference isn't
inside <kbd/>.	Replace "the Page Down key" with "<kbd>Page Down</kbd>" to both
mark it up correctly and mention it in a style that's more consistent with how
we mention keyboard shortcuts elsewhere.

> > >+    <p>Click the <em>Check Now</em> button to manually perform a check for
> > >+      updates to &brandShortName; and installed extensions.</p>
> > 
> > How about adding "appropriate" before "<em>Check Now</em>" and changing the
> > "and" near the end to "or"?
> Sure. I'd also like to mention themes, how about:
>     <p>Click the appropriate <em>Check Now</em> button to manually perform a
>       check for updates to &brandShortName;, extensions, or themes.</p>

s/, extensions, or/ or extensions and/

The user can't search for *only* themes or *only* extensions, so we should
group them together.

> > >                 <rdf:li><rdf:Description ID="saving:page" nc:name="Web Page"
> nc:link="using_firebird.xhtml#saving_all_or_part_of_a_page"/></rdf:li>
> > >-                <rdf:li><rdf:Description ID="saving:form" nc:name="Form"
> nc:link="prefs.xhtml#saved_form_information"/></rdf:li>
> > >+                <rdf:li><rdf:Description ID="saving:form" nc:name="Form"
> nc:link="prefs.xhtml#saved_forms"/></rdf:li>
> > >                 <rdf:li><rdf:Description ID="saving:passwords"
> nc:name="Passwords" nc:link="prefs.xhtml#saved_passwords"/></rdf:li>
> > 
> > The nc:link for Form should be "prefs.xhtml#privacy_saved_forms", and for
> > Passwords it should be "prefs.xhtml#privacy_passwords".
> Oops, fixed.

Well, more or less.  The former is corrected, but the latter got changed to
"prefs.xhtml#privacy_saved_passwords" instead of
"prefs.xhtml#privacy_passwords".

> > > <p>To remove a cookie from the list, select it and click <em>Remove Cookie</em>.
> > >+  Select a site to remove all cookies of that site. To wipe all cookies, click
> > 
> > The instructions on removing all cookies for a site are somewhat misleading,
> > because you have to select it first and then click the Remove Cookies button.
> OK, how about:
>   To remove all cookies of a site, select the site and click <em>Remove
>   Cookie</em>.

s/all cookies of a site/a site's cookies/
s/Cookie/Cookie(s)/

> > By the way, when you post a patch responding to these issues, please mention
> > how you dealt with each of them.  I'm not looking forward to another review of
> > this magnitude if it can be avoided by only looking at the parts that change.
> Here you are :)
> Thanks for the thorough review!

No problem.  Next time, tho, you get to be the one to manually test every item
in the ToC and Index trees.  :-P

> Please have a look at Privacy-Sanitize Settings. Should we move that down, to
> the end of the Privacy section? But it corresponds to the sentence before it,
> which describes the "Clear" button.

To be honest I'm not really sure.  There's a certain elegance of flow the way
it is now, but a good deal of it goes away when you consider that immediately
following it there's the instructions on how to access prefs within each
section.  Because this patch is a big blocker to other work now, let's leave it
alone and deal with it in a followup patch (should we decide to do one).

Anyway, assuming you agree with the above changes, r=me.  Feel free to check
this in with the above modifications.
Attachment #178118 - Flags: review?(jwalden+fxhelp) → review+
Oops, forgot to comment on the CSS in the patch to hide screenshots.  I'm not
sure whether it's a good idea or a bad idea right now, so let's let it sit on
trunk for a bit, and then when we've had a chance to see how well it works we'll
revisit this and either remove the screenshots entirely or remove the CSS to
hide them and generate new screenshots.
> > How's this:
> >   <p>&brandShortName; also allows you to clear certain or all items by pressing
> >     the &accelKey;+&shiftKey;+<kbd>Del</kbd> keyboard shortcut, or selecting the
> >     <em>Tools &gt; Sanitize &brandShortName;...</em> menu item, or, if you
> >     choose so, by closing &brandShortName;. Specify these options in the
> >     <em>Sanitize Settings...</em> dialog.</p>
> > (How can I improve the "if you choose so" part?)
> 
> Hmm.  Better, I think, but a tad on the wordy side.  Consider this, and let me
> know what you think:
> 
>   <p>&brandShortName; also allows you to clear certain or all items
>     by pressing &accelKey;+&shiftKey;+<kbd>Del</kbd> keyboard
>     or selecting <span class="menuPath">Tools &gt; Sanitize
>     &brandShortName;...</span>.  You can also change your
>     &pref.singular; to make &brandShortName; clear your data when
>     you close &brandShortName;.  &pref.singularCaps; for this are in the
>     <em>Sanitize &brandShortName;...</em> dialog.</p>
Sure, but without "keyboard", I guess, and
s/your &pref.singular;/the &pref.singular;/
s/&pref.singularCaps; for this are/&pref.pluralCaps; for this are/.

> > "You have to enter your current master password to do this."?
> 
> Let's add "will" before "have" here; I think it fits a little better with the
> style of how actions that can be performed within the UI are described.
Fixed.

> >       To purge the list,
> >       click the <em>Clear Download History Now</em> button below, or the
> >       <em>Clean Up</em> button in the Download Manager.</p>
> 
> The comma after "below" should be removed, because we're mentioning two
> separate actions that are described with only minimal modification.
Fixed.

> >   Web pages are usually displayed in the font and size specified here. The
> >   web pages' authors can choose different fonts and sizes, if you allow that
> >   in the Advanced Fonts Dialog.</p>
> 
> This feels kludgy.  How about:
> 
>   Web pages are usuallly displayed in the font and size specified
>   here.  However, web pages may override these choices unless you
>   specify otherwise in the Advanced Fonts dialog.
That's better.

> > > Two things: change "an" to "a" right before "user-selected",
> > Why? "user-selected" starts with a vowel sound, so I need to use
> > "an" instead of "a".
> As discussed on IRC, make the switch to "a" here.
Fixed.

> One other thing I didn't catch here before: the Page Down reference isn't
> inside <kbd/>. Replace "the Page Down key" with "<kbd>Page Down</kbd>" to
> both mark it up correctly and mention it in a style that's more consistent
> with how we mention keyboard shortcuts elsewhere.
Fixed.

> > Sure. I'd also like to mention themes, how about:
> >     <p>Click the appropriate <em>Check Now</em> button to manually perform a
> >       check for updates to &brandShortName;, extensions, or themes.</p>
> s/, extensions, or/ or extensions and/
> The user can't search for *only* themes or *only* extensions, so we should
> group them together.
Yeah, that's more precise.

(firebird-index1.rdf)
> Well, more or less.  The former is corrected, but the latter got changed to
> "prefs.xhtml#privacy_saved_passwords" instead of
> "prefs.xhtml#privacy_passwords".
Bah. I just added "privacy", but didn't look up the id in prefs.xhtml.

(cookies.xhtml)
> >   To remove all cookies of a site, select the site and click <em>Remove
> >   Cookie</em>.
> s/all cookies of a site/a site's cookies/
> s/Cookie/Cookie(s)/
Wow, I didn't notice the the UI is different when you click on a folder
containing one cookie (Remove Cookie), or multiple cookies (Remove Cookies).
Oh, and s/Cookie tab/Cookies tab/.

> > Thanks for the thorough review!
> No problem.  Next time, tho, you get to be the one to manually test every item
> in the ToC and Index trees.  :-P
Come on, that was easy compared to me cleaning up index and toc before 1.0. :-))

> > Please have a look at Privacy-Sanitize Settings. Should we move that down, to
> > the end of the Privacy section? But it corresponds to the sentence before it,
> > which describes the "Clear" button.
> 
> To be honest I'm not really sure.  There's a certain elegance of flow the way
> it is now, but a good deal of it goes away when you consider that immediately
> following it there's the instructions on how to access prefs within each
> section.  Because this patch is a big blocker to other work now, let's leave it
> alone and deal with it in a followup patch (should we decide to do one).
Definitely agreed.

> Anyway, assuming you agree with the above changes, r=me.  Feel free to check
> this in with the above modifications.
Working on it!
Let's party :)

Checking in mozilla/browser/components/help/locale/en-US/prefs.xhtml;
/cvsroot/mozilla/browser/components/help/locale/en-US/prefs.xhtml,v  <-- 
prefs.xhtml
new revision: 1.28; previous revision: 1.27
done
Checking in mozilla/browser/components/help/locale/en-US/firebird-toc.rdf;
/cvsroot/mozilla/browser/components/help/locale/en-US/firebird-toc.rdf,v  <-- 
firebird-toc.rdf
new revision: 1.22; previous revision: 1.21
done
Checking in mozilla/browser/components/help/locale/en-US/firebird-index1.rdf;
/cvsroot/mozilla/browser/components/help/locale/en-US/firebird-index1.rdf,v  <--
 firebird-index1.rdf
new revision: 1.18; previous revision: 1.17
done
Checking in mozilla/browser/components/help/locale/en-US/popup.xhtml;
/cvsroot/mozilla/browser/components/help/locale/en-US/popup.xhtml,v  <-- 
popup.xhtml
new revision: 1.10; previous revision: 1.9
done
Checking in mozilla/browser/components/help/locale/en-US/tabbed_browsing.xhtml;
/cvsroot/mozilla/browser/components/help/locale/en-US/tabbed_browsing.xhtml,v 
<--  tabbed_browsing.xhtml
new revision: 1.9; previous revision: 1.8
done
Checking in mozilla/browser/components/help/locale/en-US/menu_reference.xhtml;
/cvsroot/mozilla/browser/components/help/locale/en-US/menu_reference.xhtml,v 
<--  menu_reference.xhtml
new revision: 1.21; previous revision: 1.20
done
Checking in mozilla/browser/components/help/locale/en-US/shortcuts.xhtml;
/cvsroot/mozilla/browser/components/help/locale/en-US/shortcuts.xhtml,v  <-- 
shortcuts.xhtml
new revision: 1.25; previous revision: 1.24
done
Checking in mozilla/browser/components/help/locale/en-US/cookies.xhtml;
/cvsroot/mozilla/browser/components/help/locale/en-US/cookies.xhtml,v  <-- 
cookies.xhtml
new revision: 1.10; previous revision: 1.9
done
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
I left the images and the css rule to not display them in.
It says you should double-click on a history url, which isn't true on my
nightly (Dutch) version.  Changed a colon too, looks better.

I'm not sure this is the right place for this kind of stuff, but where would I
go otherwise?
(In reply to comment #22)
> I'm not sure this is the right place for this kind of stuff, but where would I
> go otherwise?

Bugzilla's definitely the right place.  However, this bug's been closed for a
while (and I'm not sure exactly how the problem you mention is connected with
our options window documentation anyway), so this would have worked better in a
new bug.  I filed a new bug for this - see bug 298398, which I've assigned to you.
Status: RESOLVED → VERIFIED
(In reply to comment #23)
> (In reply to comment #22)
> > I'm not sure this is the right place for this kind of stuff, but where would I
> > go otherwise?
> 
> Bugzilla's definitely the right place.  

Of course.

> However, this bug's been closed for a while 

Then how come there are still some unapplied patches??

(In reply to comment #24)
> Then how come there are still some unapplied patches??

This bug contains multiple iterations of one patch, and earlier iterations were
still not quite complete and thus weren't applied (which is quite typical).  The
final patch (attachment 178118 [details] [diff] [review]) was applied with the requested modifications
(which I deemed to be easy enough to fix that another review was unnecessary). 
The final, final patch (what was actually checked in) was posted just to keep
the record straight, because the changes between it and the previous patch were
deemed to be numerous enough to make it a good idea.  The only other patch was
the one you posted, so I'm not sure where you're finding an unapplied patch here.
Comment on attachment 186962 [details] [diff] [review]
minor error in using_firef\bird.xhtml

obsolete, see bug 298398
Attachment #186962 - Attachment is obsolete: true
Steffen, that img { display: none; } is going to get removed?
It causes the autoscroll image not to show up on that page on middle-click.
(In reply to comment #27)
> Steffen, that img { display: none; } is going to get removed?

See bug 299344.
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: