Bug 340677 (pref-reorg)

update preferences panels (add anti-phishing, rationalize categories, simplify wording)

RESOLVED FIXED in Firefox 3 alpha1

Status

()

defect
RESOLVED FIXED
13 years ago
9 years ago

People

(Reporter: beltzner, Assigned: Waldo)

Tracking

(Depends on 1 bug, {helpwanted, qawanted})

Trunk
Firefox 3 alpha1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: 181b1+, )

Attachments

(7 attachments, 5 obsolete attachments)

Firefox 1.5 did a great job in refreshing the preferences panels code and simplifying some of the option text. Some of the phrasing and categorization seems awkward, and several new options have been added simply where they fit best at the time to get them in front of users.

The goal of this bug is to update the panels so they ...
- use task based strings (strong use of verb and action)
- employ user-centric language (remove development names for things)
- simplify/consolidate options (frequent self-changable options get precedence)
- some re-categorization (make them related to task groups)
     - content = viewing/rendering the content
     - privacy = stuff we remember about you
     - security = annoyances, actions that require user-consent, anti-phishing

I have designs almost completed, should be posted by EOD for discussion (will post to d-a-f when I do).
Here comes a slew of ASCII art mockups for the proposed pref-panel changes. Notes inline where required:

.-----------------------------------------------------------------.
| ######                                                          |
| ######                                                          |
|  Main                                                           |
|-----------------------------------------------------------------|
|                                                                 |
| Home Page ----------------------------------------------------- |
|   (o) Firefox Start                                             |
|   ( ) Use a Bookmark [ &bookmarkName            ]  (Choose ...) |
|   ( ) Use this page  [ http://foo.com           ] (Use Current) |
|                                                                 |
| Downloads ----------------------------------------------------- |
|   [x] Show the Downloads window when downloading a file         |
|      [ ] Close it when all downloads are finished               |
|                                                                 |
|   (o) Save files to [ Desktop                   ]  (Browse ...) |
|   ( ) Always ask me where to save files                         |
|                                                                 |
| System Defaults ----------------------------------------------- | 
|   [x] Always check to see if &prodName; is the      (Check Now) |
|       default browser                                           |
|                                                                 |
'-----------------------------------------------------------------'

 - radio buttons for home page to make it clear/easy to switch
 - multiple homepages shouldn't be the default action for "use current"
 - moved primary download options to first tab since they're frequently used
 - move network connection to advanced since it is

.-----------------------------------------------------------------.
|           ######                                                |
|           ######                                                |
|            Tabs                                                 |
|-----------------------------------------------------------------|
|                                                                 |
| When a link opens a new web page, open it in                    |
|  (o) a new tab                                                  |
|  ( ) a new window                                               |
|                                                                 |
| [x] Warn me when I close a window with multiple tabs            |
| [x] Warn me when I open lots of tabs at once                    |
| [ ] Always show the tab bar                                     |
| [ ] When I open a link in a new tab, switch to it immediately   |
|                                                                 |
'-----------------------------------------------------------------'

   - tabs are more about browsing, and one of our "killer features"
     and so they should get primacy over other options

.-----------------------------------------------------------------.
|                    ######                                       |
|                    ######                                       |
|                    Content                                      |
|-----------------------------------------------------------------|
|                                                                 |
| [ ] Block pop-up windows                       (Exceptions...)  |
| [ ] Load images automatically                  (Exceptions...)  |
| [ ] Resize large images to fit in the window
|                                                                 |
| Default font: [ Times               |v]         (Advanced ...)  |
| Default size: [ 16pt   |v]                        (Colors ...)  |
|                                                                 |
| Feeds --------------------------------------------------------- |
| When I click on a web feed                                      |
|   ( ) Always subscribe with a Live Bookmark                     |
|   ( ) Always subscribe using the Feed Reader                    |
|   (o) Ask me what to use                                        |
|                                                                 |
|   Feed Reader [ Bloglines                   |v]                 |
|                                                                 |
| File Types  --------------------------------------------------- |
| Set what actions &prodName; should automatically                |
| perform after downloading certain types of files   (Manage ...) |
|                                                                 |
'-----------------------------------------------------------------'

  - viewing content is the next-most-important thing
  - moved options about security to a new "security" pref panel
  - moved resizing option to this panel from advanced
     
.-----------------------------------------------------------------.
|                              ######                             |
|                              ######                             |
|                              Privacy                            |
|-----------------------------------------------------------------|
|                                                                 |
| History ------------------------------------------------------- |
| [ ] Remember my last [ 7 ] days of browsing                     |
| [ ] Remember what I enter in forms and the search bar           |
| [ ] Remember items that I've downloaded [until I remove them|v] |
|                                                                 |
| Cookies ------------------------------------------------------- |
| [ ] Accept cookies from sites                   (Exceptions...) |
|     [ ] Only accept cookies from sites that I visit             |
|     [ ] Always ask before accepting a cookie                    |
|                                                                 |
| (Show Cookies... )                                              |
|                                                                 |
| Passwords ----------------------------------------------------- |
| [ ] Remember passwords for sites                (Exceptions...) |
| [ ] Use a master password              (Set Master Password...) |
|                                                                 |
| (Show Saved Passwords... )                                      |
|                                                                 |
| Clear Private Data -------------------------------------------- |
| [ ] Always clear some or all of my private data                 |
|     when I close %prodName                        (Settings...) |
|                                                                 |
'-----------------------------------------------------------------'

  - cookie manager needs a "remove & block" button
  - keeping cookies for "this session only" becomes part of the "always
    clear some or all of my private data" settings
  - moved cache settings to advanced
  - removed all the "Clear Now" buttons; instead, we should optimize the
    Clear Private Data tool so that there's a single, consistent UI 
    offered for the task of wiping some or all of one's private data
  - will file followup bugs on making slight changes to Clear Private Data UI
        
.-----------------------------------------------------------------.
|                                       ######                    |
|                                       ######                    |
|                                      Security                   |
|-----------------------------------------------------------------|
|                                                                 |
| [ ] Enable Javascript                             (Advanced ...)|
| [ ] Enable Java                                                 |
| [ ] Warn me when sites try to install add-ons   (Exceptions ...)|
| [ ] Check to see if the site I'm loading is a forgery           |
|     ( ) Use a list of known forgeries                           |
|     ( ) Ask [ Google |v] to check every site I visit            |
|                                                                 |
| Warning Messages ---------------------------------------------- |
| Choose which warning messages you want to see                   |
| while browsing the web                            (Settings ...)|
|                                                                 |
'-----------------------------------------------------------------'

  - new pref panel for security settings, warnings, anti-phishing
 
.-----------------------------------------------------------------.
|                                                 ######          |
|                                                 ######          |
|                                                Advanced         |
|-----------------------------------------------------------------|
|                                                                 |
|_||General|| Network | Update | Encryption |_____________________|
|                                                                 |
| Accesibility  ------------------------------------------------- |
| [ ] Use caret browsing to control the cursor                    |
| [ ] Search for text when I start typing                         |
|                                                                 |
| Browsing  ----------------------------------------------------- |
| [ ] Use autoscrolling                                           |
| [ ] Use smooth scrolling                                        |
|                                                                 |
| Languages  ---------------------------------------------------- |
| Choose your preferred language for displaying pages  (Choose...)|                                         |                                                                 |
'-----------------------------------------------------------------'

.-----------------------------------------------------------------.
|                                                 ######          |
|                                                 ######          |
|                                                Advanced         |
|-----------------------------------------------------------------|
|                                                                 |
|_| General ||Network|| Update | Encryption |_____________________|
|                                                                 |
| Connection  --------------------------------------------------- |
| Determine how &prodName; connects to the                        |
| Internet                                         (Settings ...) |
|                                                                 |
| Cache  -------------------------------------------------------- |
| Use up to [50] MB of space for the cache                        |
|                                                                 |
'-----------------------------------------------------------------'

  - moved these settings to advanced section since they're
    really about advanced network optimization

.-----------------------------------------------------------------.
|                                                 ######          |
|                                                 ######          |
|                                                Advanced         |
|-----------------------------------------------------------------|
|                                                                 |
|_| General | Network ||Update|| Encryption |_____________________|
|                                                                 |
.                                                                 .
.                          (no changes)                           .
.                                                                 .
|                                                                 |
'-----------------------------------------------------------------'

.-----------------------------------------------------------------.
|                                                 ######          |
|                                                 ######          |
|                                                Advanced         |
|-----------------------------------------------------------------|
|                                                                 |
|_| General | Network | Update ||Encryption||_____________________|
|                                                                 |
.                                                                 .
.               (same as "Advanced > Security")                   .
.                                                                 .
|                                                                 |
'-----------------------------------------------------------------'
(Added link to dev-apps-firefox posting in the URL field)
Alias: pref-reorg
Updates from comments in d-a-f. I think we're ready to get cracking on implementation. As per mconnor, I'm assigning this to jwalden.

 - these now include the suggested defaults
 - of course, let me know if you have any questions!

.-----------------------------------------------------------------.
| ######                                                          |
| ######                                                          |
|  Main                                                           |
|-----------------------------------------------------------------|
|                                                                 |
| Home Page ----------------------------------------------------- |
|   (o) Firefox Start                                             |
|   ( ) Use a Bookmark [ &bookmarkName            ]  (Choose ...) |
|   ( ) Use this page  [ http://foo.com           ] (Use Current) |
|                                                                 |
| Downloads ----------------------------------------------------- |
|   [x] Show the Downloads window when downloading a file         |
|      [ ] Close it when all downloads are finished               |
|                                                                 |
|   (o) Save files to [ Desktop                   ]  (Browse ...) |
|   ( ) Always ask me where to save files                         |
|                                                                 |
| System Defaults ----------------------------------------------- | 
|   [x] Always check to see if &prodName; is the      (Check Now) |
|       default browser                                           |
|                                                                 |
'-----------------------------------------------------------------'

.-----------------------------------------------------------------.
|           ######                                                |
|           ######                                                |
|            Tabs                                                 |
|-----------------------------------------------------------------|
|                                                                 |
| When a link opens a new window, open it in                      |
|  (o) A new tab                                                  |
|  ( ) A new window                                               |
|                                                                 |
| [x] Warn me when I close a window with multiple tabs            |
| [x] Warn me when I open lots of tabs at once                    |
| [ ] Always show the tab bar                                     |
| [ ] When I open a link in a new tab, switch to it immediately   |
|                                                                 |
'-----------------------------------------------------------------'

.-----------------------------------------------------------------.
|                    ######                                       |
|                    ######                                       |
|                   Content                                       |
|-----------------------------------------------------------------|
|                                                                 |
| [X] Block pop-up windows                       (Exceptions...)  |
| [X] Load images automatically                  (Exceptions...)  |
| [X] Enable Javascript                           (Advanced ...)  |
|                                                                 |
| Default font: [ Times               |v]         (Advanced ...)  |
| Default size: [ 16pt   |v]                        (Colors ...)  |
|                                                                 |
| Feeds --------------------------------------------------------- |
| Feed Reader: [ Bloglines                   |v]                  |
|                                                                 |
| When I click on a web feed                                      |
|   (o) Ask me what to use                                        |
|   ( ) Always subscribe with a Live Bookmark                     |
|   ( ) Always subscribe using the Feed Reader                    |
|                                                                 |
| File Types  --------------------------------------------------- |
| Configure how &prodName; handles certain          (Manage ...)  |
| types of files                                                  |
|                                                                 |
'-----------------------------------------------------------------'

     
.-----------------------------------------------------------------.
|                              ######                             |
|                              ######                             |
|                              Privacy                            |
|-----------------------------------------------------------------|
|                                                                 |
| History ------------------------------------------------------- |
| [X] Remember my last [ 7 ] days of browsing                     |
| [X] Remember what I enter in forms and the search bar           |
| [X] Remember what I've downloaded                               |
|                                                                 |
| Cookies ------------------------------------------------------- |
| [X] Accept cookies from sites                   (Exceptions...) |
|     [ ] Always ask before accepting a cookie                    |
|     [ ] Only keep the cookies until I close &prodName;          |
|                                                                 |
| (Show Cookies... )                                              |
|                                                                 |
| Passwords ----------------------------------------------------- |
| [X] Remember passwords for sites                (Exceptions...) |
| [ ] Use a master password              (Set Master Password...) |
|                                                                 |
| (Show Saved Passwords... )                                      |
|                                                                 |
| Clear Private Data -------------------------------------------- |
| [ ] Always clear my private data when I close                   |
|     %prodName                                     (Settings...) |
|                                                                 |
'-----------------------------------------------------------------'
        
.-----------------------------------------------------------------.
|                                       ######                    |
|                                       ######                    |
|                                      Security                   |
|-----------------------------------------------------------------|
|                                                                 |
| [X] Warn me when sites try to install add-ons   (Exceptions ...)|
| [X] Check to see if the site I'm loading is a forgery           |
|     (o) Use a list of known forgeries provided by [ Google |v]  |
|     ( ) Ask [ Google |v] to check every site I visit            |
|                                                                 |
| Warning Messages ---------------------------------------------- |
| Choose which warning messages you want to see                   |
| while browsing the web                            (Settings ...)|
|                                                                 |
'-----------------------------------------------------------------'

.-----------------------------------------------------------------.
|                                                 ######          |
|                                                 ######          |
|                                                Advanced         |
|-----------------------------------------------------------------|
|                                                                 |
|_||General|| Network | Update | Encryption |_____________________|
|                                                                 |
| Accesibility  ------------------------------------------------- |
| [ ] Always use the cursor keys to navigate within pages         |
| [ ] Search for text when I start typing                         |
|                                                                 |
| Browsing  ----------------------------------------------------- |
| [ ] Use autoscrolling                                           |
| [ ] Use smooth scrolling                                        |
|                                                                 |
| Languages  ---------------------------------------------------- |
| Choose your preferred language for displaying pages  (Choose...)|                                         |                                                                 |
'-----------------------------------------------------------------'

.-----------------------------------------------------------------.
|                                                 ######          |
|                                                 ######          |
|                                                Advanced         |
|-----------------------------------------------------------------|
|                                                                 |
|_| General ||Network|| Update | Encryption |_____________________|
|                                                                 |
| Connection  --------------------------------------------------- |
| Configure how &prodName; connects to the                        |
| Internet                                         (Settings ...) |
|                                                                 |
| Cache  -------------------------------------------------------- |
| Use up to [50] MB of space for the cache                        |
|                                                                 |
'-----------------------------------------------------------------'

.-----------------------------------------------------------------.
|                                                 ######          |
|                                                 ######          |
|                                                Advanced         |
|-----------------------------------------------------------------|
|                                                                 |
|_| General | Network ||Update|| Encryption |_____________________|
|                                                                 |
.                                                                 .
.                          (no changes)                           .
.                                                                 .
|                                                                 |
'-----------------------------------------------------------------'

.-----------------------------------------------------------------.
|                                                 ######          |
|                                                 ######          |
|                                                Advanced         |
|-----------------------------------------------------------------|
|                                                                 |
|_| General | Network | Update ||Encryption||_____________________|
|                                                                 |
.                                                                 .
.               (same as "Advanced > Security")                   .
.                                                                 .
|                                                                 |
'-----------------------------------------------------------------'
Assignee: beltzner → jwalden+bmo
Whiteboard: [swag: 4d]
Status: NEW → ASSIGNED
Flags: blocking-firefox2+

Comment 4

13 years ago
Instead of saying that the Default Font Size is 16.
How about simplifying that a bit.

Hows this.

Deafult Font Size   [100 % v]

Pretty much making the fonts in % and a drop down to select the percentage.
(In reply to comment #4)
> Instead of saying that the Default Font Size is 16.
> How about simplifying that a bit.

See thread in in dev.apps.firefox (linked to from URL). That would be a separate bug, as it would require new functionality which can't be contained at the moment. Hixie will be filing a bug with his proposal.
From IRC ..:

<@Waldo> beltzner: was omission of colons from many of the labels in the options window intentional?

<@beltzner> Waldo: k, for prefs where the text is just a label that sets up indented options, please add a colon; for prefs where the text is itself an option, no colon

So that would mean ...

 - When a link opens a new window, open it in:
 - When I click on a web feed:
 - Automatically check for updates to:
 - When updates to &prodName; are found:

And then no colons for the other ones.
Assignee

Comment 7

13 years ago
I just committed the string changes for the preference window, to both branch and trunk.  Hopefully these will be what we use in 2.0, bar as few changes as possible.  I got a verbal r+a=mconnor on the changes, and I posted an announcement to m.d.l10n about the changes[1].

Note that existing strings are still present in the file but have been moved to the end of the file if they are to be no longer used.  I hope to remove these strings in a future patch to avoid confusion, but for now there are big localization notes telling localizers not to waste time localizing those strings.

1. news://news.mozilla.org:119/adudneurK5_hJA7ZnZ2dnUVZ_sednZ2d@mozilla.org
Assignee

Comment 8

13 years ago
(In reply to comment #3)
> |                                      Security                   |
...
> | Warning Messages ---------------------------------------------- |
> | Choose which warning messages you want to see                   |
> | while browsing the web                            (Settings ...)|
> |                                                                 |
> '-----------------------------------------------------------------'
...
> |                                                Advanced         |
...
> |_| General | Network | Update ||Encryption||_____________________|
> |                                                                 |
> .                                                                 .
> .               (same as "Advanced > Security")                   .

Since Advanced > Security currently includes a warning message section, that makes two of them.  I assumed the Advanced > Encryption one was being removed when organizing strings in the just-posted patch.

Also, for those not actually implementing this, the Content and Privacy panes are somewhat too large vertically, so those two panes will probably see a couple changes before the dust settles completely.

Comment 9

13 years ago
Comment on attachment 225970 [details] [diff] [review]
String-only patch as committed to branch/trunk

>Index: browser/locales/en-US/chrome/browser/preferences/advanced.dtd
>===================================================================
>+<!ENTITY updateTab.label                 "Update">
>+
>+<!ENTITY autoCheck.label                 "Automatically check for updates to:">
>+<!ENTITY enableAppUpdate.label           "&brandShortName;">
>+<!ENTITY enableAppUpdate.accesskey       "F">
>+<!ENTITY enableExtensionUpdate.label     "Installed Extensions and Themes">
>+<!ENTITY enableExtensionUpdate.accesskey "x">
>+<!ENTITY enableSearchUpdate.label        "Search Engines">
>+<!ENTITY enableSearchUpdate.accesskey    "h">
>+<!ENTITY checkNow.label                  "Check Now...">
>+<!ENTITY appCheckNow.accesskey           "N">

The "Check Now..." button was removed before 1.5 in bug 305994, and I don't see it in the proposed UI in comment #3. These last two entities should perhaps also be removed?

Updated

13 years ago
Blocks: pw-doc
*** Bug 324321 has been marked as a duplicate of this bug. ***
Assignee

Comment 11

13 years ago
(In reply to comment #9)
> The "Check Now..." button was removed before 1.5 in bug 305994, and I don't see
> it in the proposed UI in comment #3. These last two entities should perhaps
> also be removed? 

Thanks, good catch.  At the time that fix was made, l10n was pretty much frozen, so removing strings wasn't acceptable, and since in the redesign the Update tab wasn't changing, I didn't do a thorough review of the strings to check for extras and redundancies.  If I can remove the unused strings, I'll make sure to remove that one as well.

Comment 12

13 years ago
How about the following (change marked by '+')?

 | Home Page ----------------------------------------------------- |
 |   (o) Firefox Start                                             |
 |   ( ) Use a Bookmark [ &bookmarkName            ]  (Choose ...) |
 |   ( ) Use this page  [ http://foo.com           ] (Use Current) |
+|   ( ) Use a blank page                                          |
 |                                                                 |
Hm, apparently we have a sizing problem! Solutions forthcoming.

Comment 14

13 years ago
[Justification for comment 12:]

Without the proposed "Use a blank page", the most suitable way of specifying a blank home page is via "Use this page", with a code indicating a blank page (in the current release, "about:blank" is the code).

However, "Use this page" suggests "Use this **web** page", and [the lack of a page] is *not* a web page.  To most users, a web page has:

  -o- a URL beginning with "http(s)://", probably followed by "www.";
  -o- a title;
  -o- content.

So using "Use this page" for accepting a blank page is inconsistent with what most users expect.  Thus the "Use this page" field is overloaded: it then has two functions, even though its label suggests that it has only one.

Further, a special "about:blank" or "(Blank Page)" code is unnecessary complexity.  Add more of the same if we're to have a separate "Use Blank Page" command button, whose purpose is to imitate the radio button I'm proposing.

Thus there should be a "Use a blank page" radio button.
.-----------------------------------------------------------------.
|                    ######                                       |
|                    ######                                       |
|                   Content                                       |
|-----------------------------------------------------------------|
|                                                                 |
| [X] Block pop-up windows                       (Exceptions...)  |
| [X] Load images automatically                  (Exceptions...)  |
| [X] Enable Javascript                           (Advanced ...)  |
|                                       (Fonts ...) (Colors ...)  |
|                                                                 |
| Feeds --------------------------------------------------------- |
| Feed Reader: [ Bloglines                   |v]                  |
|                                                                 |
| When I click on a web feed                                      |
|   (o) Ask me what to use                                        |
|   ( ) Always subscribe with a Live Bookmark                     |
|   ( ) Always subscribe using the Feed Reader                    |
|                                                                 |
| File Types  --------------------------------------------------- |
| Configure how &prodName; handles certain          (Manage ...)  |
| types of files                                                  |
|                                                                 |
'-----------------------------------------------------------------'

So my thinking here is that having the font and size in this window really isn't all that important, especially since that functionality already exists in the "Fonts..." sub-panel. This would be my preference, if people agree with my assertion that these font options aren't really tweaked all that often.

An alternative might be to combine some of the feed reader options:

.-----------------------------------------------------------------.
|                    ######                                       |
|                    ######                                       |
|                    Content                                      |
|-----------------------------------------------------------------|
|                                                                 |
| [ ] Block pop-up windows                       (Exceptions...)  |
| [ ] Load images automatically                  (Exceptions...)  |
| [ ] Resize large images to fit in the window                    |
|                                                                 |
| Default font: [ Times               |v]         (Advanced ...)  |
| Default size: [ 16pt   |v]                        (Colors ...)  |
|                                                                 |
| Feeds --------------------------------------------------------- |
| When I click on a web feed                                      |
|   (o) Ask me what to use                                        |
|   ( ) Always subscribe with a Live Bookmark                     |
|   ( ) Always subscribe using [ Bloglines       |v]              |
|                                                                 |
| File Types  --------------------------------------------------- |
| Set what actions &prodName; should automatically                |
| perform after downloading certain types of files   (Manage ...) |
|                                                                 |
'-----------------------------------------------------------------'

I'd previously been against this idea, since the feed reader choice here is also used as the default for the "always ask" option, but it can be changed in the FeedPreview dialog, so it's acceptable if it makes implementing the entire panel more easily.

Comment 16

13 years ago
What about:

 .-----------------------------------------------------------------.
 |           ######                                                |
 |           ######                                                |
 |            Tabs                                                 |
 |-----------------------------------------------------------------|
 |                                                                 |
-| When a link opens a new window, open it in                      |
+| When a new webpage is opened, open it in                        |
 |  (o) A new tab                                                  |
 |  ( ) A new window                                               |
 |                                                                 |
 | [x] Warn me when I close a window with multiple tabs            |
 | [x] Warn me when I open lots of tabs at once                    |
 | [ ] Always show the tab bar                                     |
 | [ ] When I open a link in a new tab, switch to it immediately   |
+| [ ] Deny websites to open new tabs/windows                      |
 |                                                                 |
 '-----------------------------------------------------------------'

Changes:
Webpages are not only opened by links, also by "button clicks" in external programs, by desktop icons and so on. And it it would be strange to open a new window in a new tab. You can open a webpage in a new window or in a new tab, but not a window.

At last I added a checkbox for forcing links and new javascript-windows without a specific size to the same window. (the preference is browser.link.open_newwindow:1). This feature is rather important (keyword: more power to the user) and many people were very happy when it was included in Firefox 1.5. It would be bad to remove it in the next release without a major reason.

PS: My wording may be bad, I'm not a native english speaker. ;)

Comment 17

13 years ago
> | When a link opens a new window, open it in                      |
> |  (o) A new tab                                                  |
> |  ( ) A new window                                               |

Why does it have to be radio elements in the first place? Now that we've only got two options, why not make it a checkbox instead (text open for discussion):

  | [X] Open links in new tabs instead of new windows               |

Additionally: why not put the tab options into a group box (even without caption)? The "Tabs" pane currently looks rather unpolished in comparison with the other panes.
(In reply to comment #16)
> -| When a link opens a new window, open it in                      |
> +| When a new webpage is opened, open it in                        |
>  |  (o) A new tab                                                  |
>  |  ( ) A new window                                               |

Hm. This is a good point. Suggested wording fix, though:

 | New pages should be opened in:                                  |
 |  (o) A new tab                                                  |
 |  ( ) A new window                                               |

> power to the user) and many people were very happy when it was included in
> Firefox 1.5. It would be bad to remove it in the next release without a major
> reason.

This has been discussed repeatedly in dev-apps-firefox (see the URL link, above) and the arguments against including it are there. We're not going to be putting this checkbox back in, but the about:config option is there for power users who want to restrict web pages in this way.

(In reply to comment #17)
> Why does it have to be radio elements in the first place? Now that we've only
> got two options, why not make it a checkbox instead (text open for discussion):
> 
>   | [X] Open links in new tabs instead of new windows               |

Even worded as "[X] New pages are opened in tabs instead of windows" I find this to be less clear. Checkboxes should be used for on/off, yes/no style decisions, not for alternative interpretations of a behaviour. It just parses strangely.

> Additionally: why not put the tab options into a group box (even without
> caption)? The "Tabs" pane currently looks rather unpolished in comparison with
> the other panes.

Adding a group box because the other panes have one is adding pixels and clutter for no good reason. The other panes have them to separate groups of related preferences. All of these preferences have to do with tabs.

Comment 19

13 years ago
(In reply to comment #18)
>  | New pages should be opened in:                                  |

One could argue that every time the user clicks a link, it opens a new page.  Thus, the above suggests that *all* URL clicks will open in a new tab.  The "new window" distinction is much clearer, IMO.

Comment 20

13 years ago
.-----------------------------------------------------------------.
|                                                 ######          |
|                                                 ######          |
|                                                Advanced         |
|-----------------------------------------------------------------|
|                                                                 |
|_||General|| Network | Update | Encryption |_____________________|
|                                                                 |
| Accesibility  ------------------------------------------------- |
| [ ] Always use the cursor keys to navigate within pages         |
| [ ] Search for text when I start typing                         |
|                                                                 |
| Browsing  ----------------------------------------------------- |
| [ ] Use autoscrolling                                           |
| [ ] Use smooth scrolling                                        |
| [ ] Save session                                                | +
|                                                                 |
| Languages  ---------------------------------------------------- |
| Choose your preferred language for displaying pages  (Choose...)|            
|                                                                 |
'-----------------------------------------------------------------'

i think the new session restore feature would be a good idea.

Updated

13 years ago
Depends on: 342701
Assignee

Comment 21

13 years ago
Things not done:
- setting a bookmark as homepage doesn't work, or at least doesn't work right
- loading prefs when a bookmark is set as home page doesn't show the home page
  as a bookmark
- feed options don't work, aren't initialized from prefs (pretty much at all)
- fonts dlg doesn't include UI to configure default font
- Privacy pane JS is mostly untouched since I don't know what the UI for it will
  look like, and I don't want to do too much extra work when stuff could change
- Privacy pane UI probably mostly doesn't work
- phishing list in Security pane not filled/initialized
- many scattered XXX things to remove

Things done:
- all UI implemented (not all necessarily working) in XUL
- all other parts of the Main pane
- the Tabs pane
- the non-feed, non-font/color parts of the Content pane
- the non-phishing parts of the Security pane
- all of the Advanced pane
- lots of docs -- you won't have to read the MozillaZine KB to find out the
  meaning and values of a preference tweaked in the main UI! (non-dialog UI --
  will fix those later)

Comments welcome, but recognize there's still a bunch to do (which is why there isn't and won't be a review request for this patch)...
I suggest splitting key useCachedPhishList.label "Use a list of known forgeries provided by" in two to make localizing it a bit easier (this would help Finnish at least and i'm sure many other locales too).

ie. in English just useCachedPhishListBefore.label "Use a list of known forgeries provided by" and useCachedPhishListAfter.label "". Other locales can use the after part when necessary. The source of the cached list should of course be in between those strings.
I think we want to do concatenated strings in property files, really, which take
args to strings. I don't think that doing two strings is going to be low-risk
from an l10n point of view.
Assignee

Updated

13 years ago
Blocks: 342639

Updated

13 years ago
Whiteboard: [swag: 4d] → [swag: 4d] 181b1+

Updated

13 years ago
Blocks: 221445
No longer depends on: 342701
Assignee

Comment 24

13 years ago
Posted patch Patch for review (obsolete) — Splinter Review
There are still a few remaining issues:

- Main: setting a bookmark as home page doesn't work
- Content: feeds section not implemented
- Privacy: the "remember visited pages for...days" is implemented as I
  understand it has been requested, but I think the preference's UI/UE is
  totally wrong -- test it out and tell me what you think
- Security: the cached/uncached lists are populated from the same preference
  because there's no preference split to accommodate different providers for the
  different lists; tell me if you dislike the interaction model for setting and
  changing the master password

Some of the above issues could be fixed if another patch is really desired, but to be absolutely, completely honest, I'm utterly and completely tired of working on this bug and patch, and I'd prefer to push these issues (excepting possibly the "visited pages" preference UI) out to other bugs and other patches.
Attachment #227119 - Attachment is obsolete: true
Attachment #227924 - Flags: review?(mconnor)
Assignee

Comment 25

13 years ago
I forgot to fix about:blank, and while doing so I discovered a few other small problems, all fixed in this minor interdiff-generated patch which should be applied on top of the previous patch for testing.
Whiteboard: [swag: 4d] 181b1+ → [needs review mconnor] 181b1+
Assignee

Comment 26

13 years ago
Using a bookmark as home page now works, except for a couple problems -- first, we don't recognize this fact in non-Places builds if the "set as Bookmark" occurred before the prefwindow was just openeed because doing so involves deep RDF hacking and general pain and suffering, and second, in Places builds we recognize this somewhat incorrectly because the query to determine whether a URL is in bookmarks also checks history as well, so things aren't always quite right (we don't destroy the homepage or anything, tho).

The other problems still remain, except that I changed a little CSS in this patch to make the Main and Security pane icons not be quite so wrong (which isn't to say they don't need icons, but rather to say that the panes don't display the entire icon pane palette at once).
Attachment #227924 - Attachment is obsolete: true
Attachment #227939 - Attachment is obsolete: true
Attachment #228098 - Flags: review?(mconnor)
Attachment #227924 - Flags: review?(mconnor)

Comment 27

13 years ago
(In reply to comment #26)
> Created an attachment (id=228098) [edit]

Patched, built and tested on BeOS 1.8.1 branch.  

Security-Warning Messages-Settings button does not work under BeOS; pushing button does not open a new window.

Also, alternate themes do not display the Main and Security icons correctly - they show the entire graphic file instead of just the appropriate range of pixels for the icon.  It would be great if this could be backwards compatible with older versions.  Otherwise, theme designers will need to find a way to accommodate both this panel and the 1.5 version.  
  
Assignee

Comment 28

13 years ago
(In reply to comment #27)
> Security-Warning Messages-Settings button does not work under BeOS; pushing
> button does not open a new window.

Is anything dumped to stdout/stderr or shown in the JS console?  Frankly, this seems to me like one of the least likely places for the code to break.

> Also, alternate themes do not display the Main and Security icons correctly -
> they show the entire graphic file instead of just the appropriate range of
> pixels for the icon.  It would be great if this could be backwards compatible
> with older versions.  Otherwise, theme designers will need to find a way to
> accommodate both this panel and the 1.5 version.  

That shouldn't be difficult -- panel icons are determined by id targeting (paneMain, paneSecurity), so all they should have to do is add a few extra CSS rules which are more or less duplicates of the paneGeneral and paneDownloads ones.
Comment on attachment 228098 [details] [diff] [review]
Using a bookmark as homepage works (mostly)

+            <!--
+              The values on these radio buttons may look like l12y issues, but
+              they're not - this preference uses *those strings* as its values.
+              I KID YOU NOT.
+            -->

man, I know I meant to say this, but I'm sorry I put you through this... ;)

General notes: all significant elements should have ids for hiding/hooking with userChrome/theme hacks, 
there's a few places that you removed these


+    return preference.value;
+  },
+
+
+
+
+
+
+
+
+
+
+
+

enough newlines?

+  /**
+   * Displays a dialog in which the user can select a bookmark to use as home
+   * page.  If the user selects a bookmark, that bookmark's name is displayed in
+   * UI and the bookmark's address is stored to the home page preference.
+   */
+  setHomePageToBookmark: function ()
+  {

Without knowing the APIs, I would love to store a reference to the bookmark 
folder and open that folder in tabs, instead of just saving current state

I haven't reviewed the entire patch to my normal nitpicky standard, but this is a lot of redundant/repetitive code.  The first half dozen files were solid and bang on, so I did a fast pass on the rest and it looks good, just needs some serious testing today/tomorrow (post to MZ forums and point to hourly builds to play with this)  I'm not so concerned with backwards compat for theming, since we've broken plenty of other pieces since 1.5.
Attachment #228098 - Flags: review?(mconnor) → review+
Whiteboard: [needs review mconnor] 181b1+ → [need-a] 181b1+
Assignee

Updated

13 years ago
Blocks: 339258
Assignee

Comment 30

13 years ago
Assignee

Comment 31

13 years ago
(In reply to comment #29)
> +            <!--
> +              The values on these radio buttons may look like l12y issues, but
> +              they're not - this preference uses *those strings* as its
> values.
> +              I KID YOU NOT.
> +            -->

I'm happy to know that a few other people found this as amusing as I did.  :-)  :-\

> General notes: all significant elements should have ids for hiding/hooking
> with userChrome/theme hacks, there's a few places that you removed these

I added a mess of ids in the checked-in patch; anyone who needs a specific id should file a bug so that we can acquiesce, but I think what's there will probably be plenty.

> Without knowing the APIs, I would love to store a reference to the bookmark 
> folder and open that folder in tabs, instead of just saving current state

That's probably possible with Places, but anything of that level of complexity would involve great pain with the current bookmark system.  Furthermore, there's still the problem of re-architecting the preference to store that information (be it the folder's RDF node or a places query), regardless whether the current bookmark system or Places is in place.


> just needs some serious testing today/tomorrow (post to MZ forums and point to
> hourly builds to play with this)

Posted in the MZ forums:

http://forums.mozillazine.org/viewtopic.php?p=2356069
Assignee

Comment 32

13 years ago
(In reply to comment #27)
> Security-Warning Messages-Settings button does not work under BeOS; pushing
> button does not open a new window.

So apparently my testing efforts to test this were stupid, because I had the same problem.  Fixed in a followup commit -- does 'oncommmand' look way too much like 'oncommand' to you?
*** Bug 339258 has been marked as a duplicate of this bug. ***
No longer blocks: 339258
Assignee

Comment 34

13 years ago
I changed one instance "suspected" before checking in but forgot the second.  :-\  Posting for posterity, checking in as a trivial fix...
This seems to crash my Intel Mac build (pulled from trunk about 8:30pm PDT):

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a1) Gecko/20060705 Minefield/3.0a1

Trying to open the prefs crashes Firefox, and I see the following error in the terminal:

TypeError: Cc['@mozilla.org/browser/sessionstartup;1'] has no properties
Oh, hold the phone, I'm hearing rumours that this might be the JS 1.7 landing. The  build I grabbed from earlier in the day seems to be working (Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20060705 Minefield/3.0a1)
Assignee

Updated

13 years ago
Depends on: 343708
Problems I noticed doing some quick tests:

* when you open the prefpanel, the "Main" tab is always blank, although it does indeed save any changes you make to those prefs

* the list of feed readers is blank shows "XXX"; should pick up list of feed readers, and have a "Add ..." entry that lets you add a new one using the same method as used by the feedpreview stuff (try subscribing to a feed and you'll see what I mean)

Everything else seems to be working from what I can tell. We should make sure that the following scenarios are tested:

 1. New profile
 2. Migrated profile from Firefox 1.5.x -> Firefox 2
 3. Migrated profile from Firefox 2 -> Firefox 1.5.x
 4. Migrated profile from Firefox 1.0.x -> Firefox 2
 5. Migrated profile from Firefox 2 -> Firefox 1.0.x

At least 1, 2 and 3 need to be proven solid before we can take this for beta 1.
Keywords: helpwanted, qawanted
Choosing a bookmark as home page only works once: when prefs are reopened, there's a stream of 

Error: this.preferences.rootBranchInternal has no properties
Source File: chrome://global/content/bindings/preferences.xml
Line: 96

in the console, and a change has no effect.
Assignee

Comment 39

13 years ago
I'm working through bug 340740 stuff now to get something which is at least a stand-in for what's desired for the prefwindow; that's at about 80% right now.  The feed reading stuff should take a few hours (estimate -- there's existing code for feed readers, so I can probably adapt most of that), and the home page stuff is probably just an edge case I forgot -- estimate an hour.  This is going to get done -- just needs a little more time.
Assignee

Comment 40

13 years ago
Posted patch Feeds UI (obsolete) — Splinter Review
This patch is to some extent a hack for b1, but I can't find any real problems with it.  The problem is that the preferences don't really map to the UI well, and while there may be a better way to map things that uses the preference bindings better, I don't see it.
Attachment #228386 - Flags: review?(darin)
Assignee

Comment 41

13 years ago
(In reply to comment #38)
> Error: this.preferences.rootBranchInternal has no properties
> Source File: chrome://global/content/bindings/preferences.xml
> Line: 96

This seems to be a generic Mac problem -- I see it when I open the prefwindow for a second time, and it also seems to be causing the "blank" Main pane.  Furthermore, I'm fairly certain this isn't (at the very least entirely) my fault -- I'm seeing it in a lot of places scattered throughout the preferences window, and I see it in dialogs I haven't even touched.  I'm going to be looking into this tomorrow, and I'll be dragging some other people along as well (since I've never done Mac debugging before).
Depends on: 343832

Comment 42

13 years ago
Josh/Mark any chance you can help out debugging comment 41?

Comment 43

13 years ago
Comment on attachment 228386 [details] [diff] [review]
Feeds UI

I'm not empowered to review browser code.  I suggest asking Ben Goodger for review.
Attachment #228386 - Flags: review?(darin) → review?(bugs)
It was pointed out in the MozForum thread that the option to 'disable java' is missing.  Is this by design?  There are many users that disable javascript and java as matter of protection. 

This will draw many complaints without an easy way to turn off 'java'.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060707 Minefield/3.0a1,Firefox ID:2006070704 [cairo]
Comment on attachment 228386 [details] [diff] [review]
Feeds UI

OK. Make sure to test this well with various clean profile/no initialized prefs situations. 

Also make sure that the radio buttons don't have focus rects that flex/extend past the extent of the text. 

Do that and r=ben@mozilla.org
Attachment #228386 - Flags: review?(bugs) → review+

Comment 46

13 years ago
Phil, Jeff, is comment 38/comment 41 new with this patch?

Comment 47

13 years ago
Moving to b2 as this isn't up to par for b1 yet.
Target Milestone: Firefox 2 beta1 → Firefox 2 beta2
(In reply to comment #46)
> Phil, Jeff, is comment 38/comment 41 new with this patch?

I haven't had a chance to back out just this patch yet, but it's new since my 2006070411 build.

The one other error, a

Error: redeclaration of const Ci
Source File: chrome://browser/content/places/controller.js
Line: 43

that shows up on the first opening as well as subsequent ones, might be more significant than I thought at first, but I can't seem to see where it's being redeclared in the same scope as controller.js to rule that out as a cause.

Updated

13 years ago
Depends on: 343915

Updated

13 years ago
Depends on: 343913

Updated

13 years ago
Depends on: 343917
Assignee

Comment 49

13 years ago
(In reply to comment #48)
> Error: redeclaration of const Ci
> Source File: chrome://browser/content/places/controller.js
> Line: 43
> 
> that shows up on the first opening as well as subsequent ones, might be more
> significant than I thought at first, but I can't seem to see where it's being
> redeclared in the same scope as controller.js to rule that out as a cause.

It's possible the function-local definitions of Cc/Ci are showing through here or something vaguely nitpicky like that; it could be a problem, but I don't think it's the main problem.

I did some more looking today at it, and when the error occurs |this.preferences| refers to the <preferences/> which contains all the individual <preference/>s.  However, |this.preferences| does not have its fields as properties -- XBL methods, yes, XUL methods, yes (or at least enough that toString on them returns the function text), but not fields.  I asked sicking about it, and I think he said he vaguely remembered hearing of a similar problem once but wasn't familiar with the code enough to really be immediately helpful.  We also tried adding a constructor/destructor pair to <preferences/> to see whether they were always being called or not, and both were called on the initial, working window open/close and on the second, broken window open/close.

This wasn't happening with the old prefwindow, so there must be *some* trigger in the new code, but at this point I think the problem at least partially involves XBL.  :-\
Assignee

Comment 50

13 years ago
Neil, sicking said he thought you were familiar enough with the XBL code to be of help here -- would you have time to do so?  Alternately, would you have any suggestions on how to go about debugging this?

Comment 51

13 years ago
In the latest trunk nightly, it seems that the only way to set a blank page is to type "about:blank" or "(blank page)" in the 'Use this page' field, or to click 'Use Current' when about:blank is the current page.  Thus, apparently there is no intuitive way for the user to set a blank page, currently.  There really should be a 'Use a blank page' radio button, IMO.

Also, clicking 'Use Current' when the contents of the 'Use this page' field is "(blank page)" causes the contents of the field to change to "about:blank".

Comment 52

13 years ago
Looks like chrome://browser/content/places/controller.js might be included twice somewhere. However, a quick glance suggests that the only use of the places file is to call one function from it, which only gets a service and returns it. For that warning, you could just remove the script and get the service directly.

For the other errors, they only occur when the XUL cache is enabled, so I'd suggest it was more likely an issue with dynamic overlay loading with the cache.
(In reply to comment #52)
> Looks like chrome://browser/content/places/controller.js might be included
> twice somewhere.

Ah, "of course" (though even though I saw that in Venkman, I didn't realize I was seeing it). #ifdef mac, preferences.xul includes macBrowserOverlay.xul, which includes global-scripts.inc, which includes controller.js, and then main.xul includes it again. Getting rid of one of them clears up all the problems, not just the const redeclaration problem.

The least ugly thing that occurs to me is to make the the main.xul include #ifdef places #ifndef mac.

Comment 54

13 years ago
Just a nit, but we may wish to change "forgery" to "suspected forgery"
Assignee

Comment 55

13 years ago
(In reply to comment #54)
> Just a nit, but we may wish to change "forgery" to "suspected forgery"

It was, in a followup patch -- attachment 228251 [details] [diff] [review].  I didn't /quite/ forget when beltzner told me to make that change.  :-)  
Assignee

Comment 56

13 years ago
Posted patch Fix Mac bugSplinter Review
(In reply to comment #53)
> Getting rid of one of them clears up all the problems, not just the const
> redeclaration problem.

Ooh, shiny!

> The least ugly thing that occurs to me is to make the the main.xul include
> #ifdef places #ifndef mac.

If it's that much pain to use controller.js, we can just settle for getting the service directly -- it's only used one place anyway.
Attachment #228696 - Flags: review?
Assignee

Updated

13 years ago
Attachment #228696 - Flags: review? → review?(bugs.mano)
Assignee

Comment 58

13 years ago
Comment on attachment 228386 [details] [diff] [review]
Feeds UI

I just tested this out on Mac, and there's something wrong with it.  I retested on Linux and see similar issues, so I'm not sure what's wrong.  The feeds UI is going to need a new patch that works, not this one.
Attachment #228386 - Attachment is obsolete: true

Comment 59

13 years ago
*** Bug 344415 has been marked as a duplicate of this bug. ***
Blocks: 344565
Depends on: 344567
Depends on: 344575
Depends on: 344572
Depends on: 344578
Depends on: 344580
Depends on: 344581
Blocks: 344585
Depends on: 344588

Updated

13 years ago
Blocks: 344634

Updated

13 years ago
Blocks: 344638
Assignee

Updated

13 years ago
Depends on: 344651
The bottom borderline in Main > System Defaults is missing.
Assignee

Comment 61

13 years ago
Posted patch Patch for revised UI (obsolete) — Splinter Review
This is mostly the original redesign; however, this includes the revisions discussed after the Bon Echo meeting last week.  It gets rid of most of the issues I still had, and I think it's a definite improvement.

Testers and testing highly requested...
Attachment #229689 - Flags: review?(mconnor)
Assignee

Updated

13 years ago
Depends on: 345125
Assignee

Comment 62

13 years ago
I missed feeds.dtd and the reference to it in the appropriate jar.mn.  :-\
Attachment #229689 - Attachment is obsolete: true
Attachment #229770 - Flags: review?(mconnor)
Attachment #229689 - Flags: review?(mconnor)
Whiteboard: [need-a] 181b1+ → 181b1+
Comment on attachment 229770 [details] [diff] [review]
Previous patch plus missing file and related build-fu

There's some style bits in here, but overall this looks good and seems to be testing fine.  Let's get this on trunk first thing in the morning and we can start dealing with things as followups.
Attachment #229770 - Flags: review?(mconnor) → review+
Assignee

Comment 64

13 years ago
Previous patch in on trunk.  Given that large patches suck and that this is essentially to the point of requiring changes localized to small areas of the code, I'm closing this bug and moving all further fixes to followup bugs, which can be managed separately, which allow me to work on multiple patches at once, and which allow others to work on patches to preference window code (should they so desire).

(Note that getting this checked in on branch will also be handled in a followup bug, hence the target changes.)

(Note also that since the patch relies on not-yet-checked-in feeds code changes, the feeds UI in the new window won't work, either.  It's a known problem, and I'm waiting to be able to fix it in bug 344651...)
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 2 beta2 → Firefox 3 alpha1
Version: 2.0 Branch → Trunk
Assignee

Updated

13 years ago
Blocks: pref-branch

Updated

13 years ago
Depends on: 345599
Assignee

Updated

13 years ago
Blocks: 345769

Updated

13 years ago
Depends on: 345808

Updated

13 years ago
No longer blocks: 324321
Clearing flag, branch landing is 345516
Flags: blocking-firefox2+

Comment 66

13 years ago
This was probably hashed out in some other forum, but was it intentional that "Resize large images" be removed? I see it in the ASCII art throughout this bug, but it is gone from the Minefield pref panel.

Comment 67

13 years ago
Just wondering, why can't there be a + with sub options? I know this one is marked fixed, but there isn't really a good place to put this otherwise. The main line would have the basic option of a yes/no for whatever feature (eg:disable common annoyances) and then if you click the + you can fine tune it. Thanks for all your hard work.
(In reply to comment #66)
> This was probably hashed out in some other forum, but was it intentional that
> "Resize large images" be removed? I see it in the ASCII art throughout this
> bug, but it is gone from the Minefield pref panel.

That pref doesn't do much anymore, since we remember the resized/not resized state and do that next time.  That was a different bug, of course.

Comment 69

13 years ago
> - removed all the "Clear Now" buttons; instead, we should optimize the Clear 
>  Private Data tool so that there's a single, consistent UI offered for the task >  of wiping some or all of one's private data

Are you serious?  I need more granularity then whatever I have checked in the Clear Private Data options or are you telling me now that I each time I want to clear a specific area I have to navigate to private data settings and toggle the data points I don't want to clear and select the ones I do and then clear my private data.  I then have to revisit private data settings and then reconfigure it the way I want it to be.  This is really too MUCH simplicity and IMHO just wrong.  This is now a pain.  Please don't tell me I should look for an extension to do this.

~B

Comment 70

13 years ago
Was losing "Only accept cookies from sites that I visit" (in Privacy->Cookies) deliberate?
No longer blocks: 344585
Depends on: 344585
No longer blocks: 344565
Depends on: 344565

Comment 71

13 years ago
Small cosmetic problem that I didn't notice in the dependencies.

If I mouse over one of the "icon tabs" at the top of the prefs box and then move the cursor up to the titlebar of the prefs box the "icon tab" remains highlighted.

I'm sorry if this has already been mentioned.

Comment 72

13 years ago
(In reply to comment #71)
> Small cosmetic problem that I didn't notice in the dependencies.
> 
> If I mouse over one of the "icon tabs" at the top of the prefs box and then
> move the cursor up to the titlebar of the prefs box the "icon tab" remains
> highlighted.
> 
> I'm sorry if this has already been mentioned.
> 

I think this is probably related to bug 312566 and bug 307399. Which were fixed on trunk, but never the branch. Reading the comments, it sounds doubtful that the patch for that will land on the Firefox 2 branch.

Comment 73

13 years ago
(In reply to comment #72)
> I think this is probably related to bug 312566 and bug 307399. Which were fixed
> on trunk, but never the branch. Reading the comments, it sounds doubtful that
> the patch for that will land on the Firefox 2 branch.

Yes, it appears to be the same as 307399 (312566 is over my head). I've never noticed it on anything but the options screen because the small horizontal rule separation bar between the menu and the titlebar in the main window seems to wipe the highlight. Moving the mouse quickly enough to effectively "skip over" the horizontal rule leaves the button highlighted.

Thanks for the heads up. I wonder why this was not checked into the branch.

Updated

13 years ago
Depends on: 347535

Updated

13 years ago
Depends on: 347219

Updated

13 years ago
Depends on: 347222

Updated

13 years ago
Depends on: 347235

Updated

13 years ago
Depends on: 348276

Updated

13 years ago
Depends on: 348280

Updated

13 years ago
Depends on: 351012

Updated

13 years ago
Depends on: 351624
No longer depends on: 351012
Depends on: 352032
So this bug caused bug 349680, but I see no discussion here about that change.  Comment 70 never got answered.  What happened there, exactly, and why is that discussion not linked from the bug?
Blocks: 349680
No longer blocks: 349680
Depends on: 349680
Assignee

Comment 75

13 years ago
Sorry, I must have skimmed over the bugmail with the question when it was originally posed.  :-(  At the time I was busy landing this on branch, consolidating the list of remaining bugs, and writing patches for them against the patched branch/trunk, and my attention was pretty well occupied.

(In reply to comment #70)
> Was losing "Only accept cookies from sites that I visit" (in Privacy->Cookies)
> deliberate?

My understanding was that it was removed because "sites that I visit" is vague and requires a *lot* of explanation to describe exactly how it works (wrt. subdomains, parent domains, IP addresses, etc.):

http://lxr.mozilla.org/mozilla/source/netwerk/cookie/src/nsCookieService.cpp#1734

If I visit a site, in the general case I have no real reason to believe that anything that I see on the page is coming from a location other than that site itself.  This preference requires that I understand that not everything I see is coming from the location of the page, which if we accept the above premise is not the general case.  Consequently, if we can't explain this atypical (from the user's point of view) situation in one sentence in UI (or maybe 2-3 sentences in help docs that aren't going to be read by the people that need them), I don't think keeping it is a good idea.

It is my opinion that condensing a description of the algorithm that much is impossible, and thus removing the UI was a good idea.
Assignee

Comment 76

13 years ago
(In reply to comment #74)
> What happened there, exactly, and why is that discussion not linked from the
> bug?

The discussion in question *is* linked; you just have to skim the newsgroup thread in the URL field a bit:  see messages 4 and 9, I believe.

> and thus removing the UI was a good idea.

OK, I see.  In that case we should have some serious discussion about the default value of this option; I see some value in only allowing same-site cookies by default.  I guess we should file a separate bug for that....

Comment 78

13 years ago
> OK, I see.  In that case we should have some serious discussion about the
> default value of this option;
See bug 324397.

Comment 79

13 years ago
Add bug 355469 to list of dependencies?

Comment 80

13 years ago
Nope. That bug is about the Remember Passwords dialog, which Jeff didn't touch.
I've filed bug 362908 against the loss of the UI for blocking third-party cookies, with a proposed alternative.

Updated

13 years ago
No longer depends on: 344588

Updated

13 years ago
No longer depends on: 344585
Duplicate of this bug: 326172
(In reply to comment #68)
> That pref doesn't do much anymore, since we remember the resized/not resized
> state and do that next time.

Except we don't do that, of course.  I filed bug 390369 on the fact that there is now no user control over this behavior.
You need to log in before you can comment on or make changes to this bug.