377784 - create an easy-to-use MIME-type configuration system

Status: VERIFIED FIXED

(Firefox :: File Handling, defect)

Description

[Dan Mosedale (:dmosedale, :dmose)]

On the UI side, this is currently done using a combination of the save dialog, the content/file types preferences subdialog, the feeds pane, and the feeds preview page at least.  Much of the database is currently in RDF and duplicates some data from the OS.  We need to figure out how to minimize that (bug 372853) and (possibly, depending on time) move it to mozStorage.

Comment 1

[Jesse Ruderman]

Does this include improving the UI for configuring full-page (or even non-full-page) plugins vs downloading?

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

That's not entirely clear to me; perhaps Faaborg has some thoughts on that...

Comment 3

[Alex Faaborg [:faaborg] (Firefox UX)]

From a UI perspective, I think this interface should allow the user to control if content is loaded in a plugin vs. downloaded.

The UI should be framed around which applications and plugins the user wants to configure, and not the various ways the content is delivered (mime-type, embedded in the page, etc).

However, configuring the content handling settings for plugins might be going out of the scope of this particular PRD item.  I'll ask at the next Firefox 3 meeting.   

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Since the content-handling dialogs are tracked by another bug, we'll use this one to track the preferences changes for this.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

beltzner, I hear you're working on a UI spec for an improved version of our content handling configuration dialog.  Let me know when you've got something to show me, as that's looking to be my next task once bug 385740 is done.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Attachment: content handling prefs UI

Hey Myk,

We've decided that we'd like to go with something like Jesper's "update 3" mockup (attached, but sourced from http://temp.jesperkristensen.dk/mozilla/content_handling/prefs_proposal.html)

- drop the "Automatically find known types of content" checkbox at the bottom, as that's a microformat specific bit atm

- the uninstall button would only be there for content handlers that were installed; not sure that we're supporting that right away - dmose?

- let's not do the "expand selection on select" treatment, but instead just have the drop down be the default action; there should always be "Always ask" and "Select..." options, the latter allowing the user to pick a new local application to handle the type of content.

- the content types that should be shown are:
-- Web Feeds
-- content types that have been encountered by users
-- content types added by handlers (plugins, content handlers)
-- if the user has altered the values of download.hide_plugins_without_extensions or download.show_plugins_in_list, then show those as well, and boy-o, the list gets long

This prefpane would replace Web Feeds, and should be called Applications for now.

Comment 8

[Myk Melez [:myk] [@mykmelez]]

Digging further, it seems like the easiest and safest approach to an initial version of a Content Handling configuration interface (i.e. the Applications prefpane) might be to start with the existing Download Actions dialog and then modify it step by step until it resembles what we want.

Here's a possible set of steps for doing this:

   1. add support for protocols;
   2. switch from a tree to a richlistbox;
   3. switch Change Action from a separate dialog to an inline drop-down menu;
   4. add support for multiple applications per content type;
   5. integrate feed handling;
   6. make it a prefpane.

Additional steps we might take along the way or afterwards:

    * switch UI generation from DOM methods to templates;
    * add API for accessing the info in mimeTypes.rdf.

Comment 9

[Myk Melez [:myk] [@mykmelez]]

Erm, we'll need to add support for web handlers too.  I think step 4 is the right place for that:

    1. add support for protocols;
    2. switch from a tree to a richlistbox;
    3. switch Change Action from a separate dialog to an inline drop-down menu;
    4. add support for web handlers;
    5. add support for multiple applications per content type;
    6. integrate feed handling;
    7. make it a prefpane.

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

I like this plan; it has a set of discrete steps that ends up where we want, and, better, it's ordered so that it allows us to ship at any point along the way.

The one quibble I have: why not put "add support for web handlers" as step 2?  This seems like something that's harder to ship Firefox 3 without than the currently listed steps 2 and 3.

Comment 11

[Myk Melez [:myk] [@mykmelez]]

> The one quibble I have: why not put "add support for web handlers" as step 2? 
> This seems like something that's harder to ship Firefox 3 without than the
> currently listed steps 2 and 3.

It is more important than steps 2 or 3, but I think that all the work I do to support web handlers would be thrown away in steps 2 and 3 if I did it before then.

Comment 12

[Dan Mosedale (:dmosedale, :dmose)]

OK, sounds reasonable to me.

Comment 13

[Alex Faaborg [:faaborg] (Firefox UX)]

In Jesper's mockup, he shows generic information classifications like "location" and "event."  I think we should collapse multiple content types (protocols, mime types, and microformats) into these classifications, so the user only has to think about what application they want to use.  For instance:

"Event" = webcal, .ics, hCalendar

This way the user can simply change their Calendar applicaiton, they don't have to know what webcal, .ics and hCalendar are, find these individually, and set these individually.

Two issues we need to consider:
1) Not all applications support all types, we should know if an application is going to handle a certain content type before handing it off.  If the type isn't supported, we need to throw the unified content handling dialog.
2) We need to allow advanced users to drill down and set content handling manually, for the edge cases where users really want .ics to always go to iCal, and hCalendar to always go to Google Calendar.  Perhaps a secondary window accessed through a "details" hyperlink?

Comment 14

[Myk Melez [:myk] [@mykmelez]]

> 2) We need to allow advanced users to drill down and set content handling
> manually, for the edge cases where users really want .ics to always go to iCal,
> and hCalendar to always go to Google Calendar.  Perhaps a secondary window
> accessed through a "details" hyperlink?

Perhaps the classification list items could have twisties that open up a sub-list of content types within each classification, i.e.:

+------------------------+
| > Images               |
| > Videos               |
| V Calendar             |
|     webcal             |
|     .ics               |
|     hCalendar          |
| > Music                |
+------------------------+

Comment 15

[Alex Faaborg [:faaborg] (Firefox UX)]

I am pretty sure that the vast majority of users will not know or care what is in the sublist, so expanding the list through twisties seems a little too discoverable.  I like the overall idea of expansion though, but I think we may want to hide the sublist behind a control that says something like "details," "options," or "advanced."

Comment 16

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1 (wip): API for retrieving array of content types

Here's a work-in-progress patch that adds an attribute to nsIExternalHelperAppService for retrieving the set of content types known to the service.  This patch also modifies downloadactions.js to use the API instead of querying the datasource directly.  This is part of the way to adding support for protocols into the Download Actions dialog.

Comment 17

[Mike Kaply [:mkaply]]

When something is being sent to a web service, for instance, how is the code going to be written?

For instance, a location going to google maps. 

How are we going to define that "pipe" or allow it to be defined externally.

For point of reference, is there a way to plugin new RSS readers to Firefox?

Comment 18

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #17)
> When something is being sent to a web service, for instance, how is the code
> going to be written?
> 
> For instance, a location going to google maps. 
> 
> How are we going to define that "pipe" or allow it to be defined externally.
>
> For point of reference, is there a way to plugin new RSS readers to Firefox?

If the content in question is associated with a specific MIME type or protocol, then it'll be possible to associate a "web handler app" (web service) with that content in several ways:

1. chrome: via the nsIHandlerService being introduced in bug 387930;

2. content: via an API like the one for adding an RSS reader:
  http://developer.mozilla.org/en/docs/Adding_feed_readers_to_Firefox#Adding_a_feed_reader_from_a_web_application

3. users: via the prefpane being implemented in this bug, the existing MIME type handling dialog, and the protocol handling dialog being implemented in bug 385065 (although I'm not sure all of these will allow adding handlers as opposed to merely picking from those already added).

Comment 19

[Myk Melez [:myk] [@mykmelez]]

Over in bug 387930, I've been working on some backend infrastructure to support the changes we want to make to the frontend in this bug.  That patch is about ready to check in, but there doesn't look to be enough time to get any frontend changes in for M7, so pushing this out to M8.

Alex: another UE issue is what to do about content types for which the user has said to be "always asked" (or for which the user hasn't made a decision yet, since we always ask the user by default).

Currently, the Download Actions dialog doesn't show these at all, so there's no way for users to change any settings for "always ask" types in advance (or even know that there are settings for them in the first place).  They have to wait until they actually encounter such content and then twiddle the setting in the UCTH dialog.

It seems to me that we should expose "always ask" content types along with all the others and provide a mechanism for twiddling the "always ask" flag in the Applications prefpane.

Comment 20

[Myk Melez [:myk] [@mykmelez]]

dmose asked me to come up with some SWAGs for the time it'll take to complete the tasks in this bug.  Here's what I'm thinking:

hrs task
 5* 1. add support for protocols;
10  2. switch from a tree to a richlistbox;
 5  3. switch Change Action from a separate dialog to an inline drop-down menu;
 5  4. add support for web handlers;
10  5. add support for multiple applications per content type;
10  6. integrate feed handling;
 2  7. make it a prefpane.

*remaining; a bunch of back-end work for this was done in M7

Comment 21

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2 (wip): partial support for protocols

Here's a patch that implements partial support for protocols along with tests to make sure that I didn't break anything along the way (specifically, the tests make sure that the principle data structure in downloadactions.js, which that script generates from the datasource and from which it builds the tree of download actions, remains the same across changes to the implementation).

Comment 22

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v3: step 1: support for protocols

Here's a patch against the trunk that implements step 1 of this bug, which is to add support for local protocol handlers to the Download Actions dialog.  Display, editing, and removal of local protocol handlers are all supported.

In the process, I also moved all the RDF-related code from the dialog into the handler service, cleaned up some code, fixed some bugs, commented some code, and added some tests.

Not sure whether to have dmose and biesi review individual patches for each stage of development or only the final patch.  In theory this patch could even be broken up further into subpatches (f.e. the API changes to the handler service could be a separate patch).

It's also not clear if dmose and biesi are the right reviewers for some of this code.  They clearly are for the API changes, but perhaps someone else makes more sense for the dialog code.

Note: this patch creates a directory, uriloader/exthandler/tests/browser/, that does not yet exist on the CVS server.  I used cvsdo to make the diff, and it seems to have created a patch that can successfully be applied, but let me know if you run into problems.

Note: to run the mochitest tests associated with this patch, build as usual (mochitest is enabled by default), then do this:

cd _tests/testing/mochitest
perl runtests.pl --browser-chrome --autorun

Comment 23

[Myk Melez [:myk] [@mykmelez]]

Folks have expressed an interest in smaller patches, so I'm going to split the current patch into pieces.  Among other things, this'll let me more accurately target reviewers by section of code, since the folks reviewing the backend changes are probably not the same ones who should be reviewing the frontend ones.

Comment 24

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v4: step 1 minus the backend changes

Here's a step 1 patch without the backend pieces, which I've split off into bug 391150 and bug 391152 (necessary prerequisite patches attached to both those bugs).

This patch is mostly changes to files in browser/component/preferences/.  The only exceptions are a minor tweak of browser/locales/en-US/chrome/browser/preferences/downloadactions.dtd and a small change to toolkit/mozapps/preferences/actionsshared.js (which I suspect should be in browser/components/preferences/ anyway).

Given that the changes are all in the browser frontend code, requesting review from mconnor instead of the biesi/dmose combo I've been using for the backend changes.

In this patch I also moved the frontend MochiTest tests from uriloader/exthandler/tests to browser/components/preferences/tests/, which seems a more place for them.

Note: I created part of this patch with cvsdo to avoid having to create browser/components/preferences/tests/ on the CVS server pre-review.  It *should* apply cleanly to a tree, creating the tests directory and adding the files within it.

Comment 25

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v5: work in progress - switching to richlistbox

Here's a work in progress that switches the download actions dialog to a richlistbox.

Some notes and issues:

1. The richlistbox widget doesn't do multi-select, so by switching to a richlistbox, we're giving up the ability for a user to select multiple items from the list and delete them all at once.

2. The richlistbox widget doesn't have any concept of columns (bug 367843, there's a patch ready for review, but enn isn't sure he wants richlistboxes to have this capability), so I'm simulating them with equal-width boxes.

3. Because it doesn't have any concept of columns, the richlistbox widget doesn't have any concept of column headers.  mano checked in basic support for a single listheader (bug 376815), and I'm using one of those with two treecols inside it to simulate headers for my simulated columns.

(This triggers "JavaScript error: chrome://global/content/bindings/tree.xml, line 1249: tree.columns has no properties" when you click on the column headers, since the treecols expect to be inside a tree, and I haven't figured out how to suppress the event.)

4. Because of bug 391740, the column headers appear at the bottom of the richlistbox rather than at the top.

5. I'm reaching the point where I've almost completely rewritten downloadactions.js and have significantly rewritten downloadactions.xul, so I'm wondering if it makes more sense to move the new code into new files like handlers.js and handlers.xul, since the blame from the final patch won't be particularly instructive.

6. The "Remove Action" and "Change Action..." buttons are still part of the dialog, and you still have to select an item and then press "Change Action..." (or double-click on the item) to modify it.  I haven't hooked up the inline drop-down menu for making changes yet, although I display a rudimentary version of the menu when you select an item.

Comment 26

[Brian King [:kinger]]

FYI... for a project I am working on, we had similar needs, i.e. a list widget with a header row, in-built scrolling, columns that align with dynamic content, etc. All of the existing XUL widgets had shortcoming, more details at:

http://groups.google.com/group/mozilla.dev.tech.layout/browse_frm/thread/ce3734235674b8fd/077530a4380600d0?tvc=1&q=XUL+Tables+%2F+Columns+%2F+Listbox#077530a4380600d0

I even adapting a multi-column richlistbox patch in bug 367843, but it came up short. In the end we wrote a hybrid HTML table with XUL mixed in. Not elegant, and highly tailored to our needs so not reusable. 

Comment 27

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v6 (wip): partial implementation of drop-down menu

Here's a partial implementation of a drop-down menu for selecting the application to handle a given type.

Comment 28

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v7 wip: drop-down menu mostly done

In this patch the drop-down menu mostly works.  It shows most options (except for selecting a new app to use), and selecting an app (or other action like "Save to Disk") causes your selection to get stored and used.  But it still needs some work, particularly getting Save to Disk working, and there's a bunch of code clean-up as well that can happen now.

Comment 29

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v8 wip: support for app selection, multiple apps, and web apps

> In this patch the drop-down menu mostly works.  It shows most options (except
> for selecting a new app to use), and selecting an app (or other action like
> "Save to Disk") causes your selection to get stored and used.  But it still
> needs some work, particularly getting Save to Disk working, and there's a bunch
> of code clean-up as well that can happen now.

This patch includes app selection, support for multiple apps, and support for web apps.  It does not yet include a bunch of necessary cleanup.

Comment 30

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v9: full base implementation with code cleanup

Here's a basic implementation of most of the requested functionality, including support for protocol handlers, web handlers, and multiple applications.  The interface is now a prefpane, and I have CVS removed all the old code (which accounts for a large proportion of the size of this file).

This patch doesn't integrate feed types, it doesn't group types into categories, it doesn't display icons (in most cases), and it doesn't list applications in MRU order, but I think this patch is far enough along that it's ready for review, and we're better off reviewing now and integrating those items as followup patches.

gavin, mconnor mentioned that you're the go-to guy for reviews this week in his absence, which is why I'm requesting review from you.  Note: to test this code, apply the patch in bug 393492 before rebuilding, and be aware of bug 391740.

Alex, you're the go-to guy for UI review, so requesting that from you.  Some questions that'll need to be addressed include how to remove an entry from the list or whether it should even be possible (the old code had a "Remove" button at the bottom of the window; in this version I've added a "Remove Entry..." item to the drop-down menu), whether or not to include the filtering mechanism from the old code (I've left it in for now), and what to call the various non-application options in the drop-down menu (i.e. "Save to Disk" and "Another Application...").

Comment 31

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v10: minor updates to patch v9

Changes in this version of the patch:

1. doesn't remove the column headers when rebuilding the list;
2. adds some comments referencing code for getting icons for local and remote apps (something to do in a later version of the patch or a followup patch);
3. doesn't show the "Save to Disk" option for protocol handlers (it doesn't make sense for them);
4. move the XBL that customizes the list items into preferences.xml to work around bug 393953 in the overlay loading code;
5. removes an unnecessary reference to the preferences bundle in the prefpane overlay (preferences.xul provides this for all prefpanes);
6. crops type and action descriptions so long descriptions don't cause the listbox to overflow the dialog;
7. removes an errant reference to preferences.js, which doesn't exist;
8. sets the message at the top of the prefpane to the one displayed in the mockup, and separates that message from the one that displays when a filter is active;
9. adds the necessary style to display a prefpane icon (currently just uses the feed icon).

Comment 32

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v11: minor update to patch v10

This patch just fixes one bug I found while creating screenshots of the prefpane.

Comment 33

[Myk Melez [:myk] [@mykmelez]]

I put up some screenshots of the new prefpane on Linux at the following URL:

http://people.mozilla.com/~myk/bug/377784/

Mac and Windows screenshots to follow.

Comment 34

[Mike Connor [:mconnor]]

Comment on attachment 278614 [details] [diff] [review]
patch v11: minor update to patch v10

ui-r=me, though please fix the styling on Mac, the select has white text on grey

looking pretty great, Alex might have nits though

Comment 35

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v12: feed handling, icons, misc fixes

This patch integrates feed handling into the Applications prefpane.  With this patch, "Web Feed" is just one of the content types in the list, as was shown in the mockup.

This patch also adds a variety of mechanisms for obtaining icons for local and web handler apps and makes some misc appearance and functionality improvements.

(In reply to comment #34)
> (From update of attachment 278614 [details] [diff] [review])
> ui-r=me, though please fix the styling on Mac, the select has white text on
> grey

So far I haven't been able to figure out why it does that.  Note that if you click on the menu everything goes back to normal until you click outside of it again.  I'll have to dig deeper on this issue.

I also can't figure out how to get the OS to size the Preferences dialog to accommodate the list headers.  At the moment it just cuts them off.  Of course, the headers should be at the top of the list, not the bottom, but presumably when that bug gets fixed the dialog will just cut off the last item.

There are still a number of UX issues to address.  In particular, it would be good to review all the strings in preferences.dtd and preferences.properties to make sure they make sense.

It'd also be useful to put together a matrix of every possible action (save to disk, open internally, use plugin, use helper app, use OS default app, always ask) with every possible kind of content (MIME type, protocol scheme, feed) to identify which actions users should be able to take with which content types.

It'd also help to put together something that specifies when we should and shouldn't show a given content type.

And there's the question of how to handle removing an entry.  Currently it's an item in the drop-down menu, but that doesn't feel right.  Perhaps we could put an [X] at the far right of the entry (maybe only for selected entries).  Or we could leave it out entirely but enable the "always ask" option for all content types.  Right now we only enable it for the feed type for compatibility with the old UI.

Comment 36

[Myk Melez [:myk] [@mykmelez]]

Thunderbird also uses a Download Actions dialog for configuring attachment handling.  It looks like much the same code as Firefox's.  Perhaps they'd like to use this new one.  Maybe there's even a way to share it via toolkit/mozapps/, although that could be tricky, given that it's an integrated prefpane, not a separate dialog.

cc:ing mscott so he knows about this work.

Comment 37

[Robert Strong (they/them - no direct email)]

myk, I'm curious if the sizing on Windows / Linux is better now when opening prefs to a different view and then selecting this view? Bug 283697 should have made it behave better. Also, bug 394666 should make it better still.

Comment 38

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #37)
> myk, I'm curious if the sizing on Windows / Linux is better now when opening
> prefs to a different view and then selecting this view? Bug 283697 should have
> made it behave better. Also, bug 394666 should make it better still.

I've been using a build that's several days old.  I'll rebuild with the latest source and see if that makes things any better.

Comment 39

[Robert Strong (they/them - no direct email)]

If there are still problems you could also try the patch in bug 394666 which should make it better still

Comment 40

[cmtalbert]

I've attempted V12 version of the patch.  I realize this is a work in progress, but here are my observations:
* The application pane flickers spastically as it loads all of the items into itself
* When it is sitting with an empty search bar, the scroll bar only scrolls through the "a" entries, not all entries
* The oversized pref pane that is generated extends off my screen and cannot be resized (I'm running on a mac) Screen Shot 4.
* If I search for a specific protocol, then the protocol is listed.  I can select the default handler, but I cannot re-set the handler to "ask me what to do about this"
* If I search for a specific protocol, then the protocol is listed, and there are two buttons or headings or something at the bottom of the text window.  They are poorly clipped and I can't see what they are. Screen Shot 1.
* When an entry is selected, the application goes to a white text on light gray background.  I have decent eyes and it's really hard to read. Screen Shot 2.
* General Usability (probably should be follow on bug): I don't think that a non-technical user is ever going to be able to use this, because they need to know the protocol name in order to change or modify the behavior.  Very few people know what a protocol is, and fewer will know that they must find "webcal" to change how "online calendars" are handled.  Of course, you do map "calendar" to ICS file, so that is a step in the right direction.
* Clicking off the "Applications" panel to some other pref panel also causes the spastic flickering.  
* Click on the applications panel, search for something, switch to a different preference pane, click back on the applications panel. Now, the applications pane remains its previous size (rather small).  The list of items flows through the dialog, and is not scrollable and does not have an end. Screen shot 3.

Comment 41

[cmtalbert]

Attachment: Screenshot 1 - Mac Observation

Comment 42

[cmtalbert]

Attachment: Screen Shot 2 Mac Observations

Comment 43

[cmtalbert]

Attachment: Screen Shot 3 Mac Observations

Comment 44

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v13: fixes two issues gavin noticed on windows

Gavin noticed two issues on Windows: 

First, if there are many items in the list, and the Applications prefpane is the first one displayed when you open the Preferences dialog, then the dialog will be very tall.

Second, the dialog was showing a bunch of entries for plugin types that don't have file extensions, even though the browser.download.hide_plugins_without_extensions preference was set to true.

Those issues are fixed in this version of the patch.

ctalbert: thanks for the report!  I'm going to take a look at these Mac issues next.

Also, I sat down with faaborg yesterday, and we walked through the functionality, doing a UI review.  He identified a set of issues, none of which block checkin for M8.  I took notes on paper, and I'm going to post those as soon as I get a chance to type them in.

Comment 45

[Myk Melez [:myk] [@mykmelez]]

While I'm waiting for my Mac build to compile, I managed to type these up.  Here's the list of nits I compiled in the review with faaborg:

* We should not provide a way to remove entries from the list, so remove the
  "Remove this Entry..." item from the drop-down menu of actions.

* We should sort so that the Web Feed item is on top followed by the rest of
  the entries in alphabetical order by type name.

* We should not provide options for users to sort the list in other ways.

* The "always ask" option for feeds, which is currently called "Show me a
  preview and ask me what to do", should be called "Preview in Firefox".

* The "handle internally" option for feeds, which is currently called "Live
  Bookmarks", should be called "Live Bookmarks in Firefox".

* The type icon shouldn't change size when you select an entry.

* The order of actions for feeds should be: Preview in Firefox, Live Bookmarks
  in Firefox, [separator], [web and local apps], Choose Application...

* For types you can save to disk, the "Save to Disk" item should appear at the
  end of the list, after "Choose Application...".

* The clear button should become an "x" button inside the search field on the
  right-hand side.

* We should have "help" documentation for the prefpane.

* We should unhide types set to "always ask" and let the user make decisions
  about those.

* We should unhide the "always ask" action and let the user decide to "always
  ask" about a type.

* The name of the "always ask" action should be "Ask" (except for feeds, where
  it's "Preview in Firefox").

* Remove the intro text at the top of the prefpane.

* Plugin names should be postfaced with the text "(in Firefox)" to make it
  clearer that they open inside Firefox.

* We shouldn't filter on hidden stuff (MIME type, file extension).

* We should group types and let users change the preference for all types in a
  group at once.

* Groups should have a "Details" link that expands the group to show individual
  members for finer-grained control.

* Until we have type grouping, we should detect duplicate type names for MIME
  types and add the MIME type after the name for duplicates.

* We should ship a hard-coded list of names for known types (especially
  protocol schemes) that we think OSes aren't going to know the names of.

Also, there's an item on my list that just says "list alignment", and I have no idea what that means, but maybe faaborg remembers.

Comment 46

[Mike Connor [:mconnor]]

(In reply to comment #45)
> While I'm waiting for my Mac build to compile, I managed to type these up. 
> Here's the list of nits I compiled in the review with faaborg:
> 
> * We should not provide a way to remove entries from the list, so remove the
>   "Remove this Entry..." item from the drop-down menu of actions.

I'm not sure why, but I'm ok with this for now.

> * We should sort so that the Web Feed item is on top followed by the rest of
>   the entries in alphabetical order by type name.

Why not mailto and others?  Kinda hard for me to justify feeds getting prime placement except to aid transitioning users...

> * We should not provide options for users to sort the list in other ways.

I could go either way on this.

> * For types you can save to disk, the "Save to Disk" item should appear at the
>   end of the list, after "Choose Application...".

Hmm, I'm not sure that's right.  I save things to disk quite a lot. :)

> * The clear button should become an "x" button inside the search field on the
>   right-hand side.

We need the search textbox widget to do all of this, instead of continuously reinventing this wheel.

> * We should have "help" documentation for the prefpane.

yes, non-blocking.

> * We should unhide types set to "always ask" and let the user make decisions
>   about those.
> 
> * We should unhide the "always ask" action and let the user decide to "always
>   ask" about a type.

What types do we have that will be set to always ask?
 
> * We should group types and let users change the preference for all types in a
>   group at once.

probably true, but hard to do without some sort of list of equivalent types, and that's likely to be incomplete in a lot of places.  But worth trying.

> * We should ship a hard-coded list of names for known types (especially
>   protocol schemes) that we think OSes aren't going to know the names of.

This sounds "fun" but probably useful.

Comment 47

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 279771 [details] [diff] [review]
patch v13: fixes two issues gavin noticed on windows

I found a few issues while testing that I mentioned on IRC, I think you have a fix for all of them already. Some additional comments:

global nit: I saw "switch(" in a few places, please add a space?

>Index: browser/components/preferences/applications.js

>+HandlerInfoWrapper.prototype = {

>+  enablePluginType: function() {
>+    disabledPluginTypes =
>+      disabledPluginTypes.filter(function(v) { return v != type });

You could use an expression closure here:
disabledPluginTypes = disabledPluginTypes.filter(function(v) v != type);

>+  get smallIcon() {
>+  get largeIcon() {

Perhaps worth factoring out the common code for these two?

>+var feedHandlerInfo = {

>+  get possibleApplicationHandlers() {

>+    // Add the "selected" local application, if there is one and it's different
>+    // from the default handler for the OS.  Unlike for other types, there can
>+    // be only one of these at a time for the feed type, since feed preferences
>+    // only store a single local app.
>+    var preferredAppFile = this.element(PREF_FEED_SELECTED_APP).value;
>+    if (preferredAppFile && preferredAppFile.exists()) {
>+      let preferredApp = getLocalHandlerApp(preferredAppFile);
>+      let defaultApp = this._defaultApplicationHandler;
>+      if (!defaultApp || !defaultApp.equals(preferredApp))
>+        handlerApps.appendElement(preferredApp, false);
>+    }

This could just be me misunderstanding how this code works, but don't we want to always add the default handler, should the user want to choose it? Ah, I guess the code in rebuildActionsMenu takes care of that, and possibleApplicationHandlers is only supposed to include non-default handlers?

>+  // XXX Shouldn't we memoize this value so we don't have to keep retrieving it
>+  // over and over again?
>+  get _defaultApplicationHandler() {

Sounds like a good idea, followup bug? :)

>+  // What to do with content of this type.
>+  get preferredAction() {

>+      // If the action is "ask", then alwaysAskBeforeHandling will override
>+      // the action, so it doesn't matter what we say it is, it just has to be
>+      // something that doesn't cause the controller to hide the type.
>+      case "ask":
>+      default:
>+        Ci.nsIHandlerInfo.handleInternally;

Missing return?

>+  _getVisibleTypes: function() {

>+      // XXX If the type has a plugin, should we also check the "suffixes"
>+      // property of the plugin?

>+      // XXX "handledOnlyByPlugin" doesn't mean there isn't an OS-default
>+      // handler for the type, it only means there isn't a user-configured
>+      // handler.  Don't we want to give the user the option to switch to
>+      // the OS-default handler for these types?

>+      // XXX Shouldn't we show these types to give the user a chance
>+      // to reenable the plugins?

Worth filing a followup bug for these?

>+      // Don't display entries for types whose preferred action is alwaysAsk.
>+      // alwaysAsk is actually an obsolete preferred action (we now keep track
>+      // of that state separately via the alwaysAskBeforeHandling flag),
>+      // and we shouldn't see it anymore, and it's not clear why we're checking
>+      // for it here, but the old code did that, so for now we're doing it too.
>+      if (handlerInfo.preferredAction == Ci.nsIHandlerInfo.alwaysAsk)
>+        continue;

This sounds like it would lead to bugs where people can't change the action for a download without hand-editing mimeTypes.rdf. Is that not the case?

>+  _describePreferredAction: function(aHandlerInfo) {

>+    switch(aHandlerInfo.preferredAction) {

>+      case Ci.nsIHandlerInfo.handleInternally:

>+        // For other types, handleInternally looks like either useHelperApp
>+        // or useSystemDefault depending on whether or not there's a preferred
>+        // handler app.

>+        // XXX Why don't we say the app will handle the type internally?
>+        // Is it because the app can't actually do that?  But if that's true,
>+        // then why would a preferredAction ever get set to this value
>+        // in the first place?

What was the rationale behind the first comment, given the second?

>+  _sortTypes: function(aTypes) {
>+    // FIXME: strict warning: function does not always return a value.

Just make it "return undefined;" ?

>+  onFilterKeyPress: function(aEvent) {
>+    if (aEvent.keyCode == 27) // ESC key

Use KeyEvent.DOM_VK_ESCAPE

>+  onSelectAction: function(event) {

>+      // XXX Is there any value in leaving the existing preferred app,
>+      // if any, as it is when we set the preferred action to something
>+      // other than useHelperApp?

Perhaps worth a followup (noted in the XXX comment)? If this leaves the existing preferred app in the list, I think we should leave it.

>+  chooseApp: function(aEvent) {

>+      // XXX Should we not stop propagation above and then not store here,
>+      // relying on the normal handler to store?
>+      handlerInfo.store();

I'd remove this XXX and leave things as-is, I think it's cleaner and easier to understand that way.

>+  _getIconURLForWebApp: function(aWebAppURL) {

Could we use the faviconservice for this?

>+  _getIconURLForSystemDefault: function(aHandlerInfo) {

>+    if (aHandlerInfo instanceof Ci.nsIMIMEInfo &&
>+        aHandlerInfo instanceof Ci.nsIPropertyBag) {
>+      try {
>+        var url = aHandlerInfo.getProperty("defaultApplicationIconURL");
>+        if (iconURL)
>+          return iconURL + "?size=16";

Looks like you meant to use "url" here? Was this not tested?

>Index: browser/components/preferences/handlers.css

I really appreciate the comments in these CSS files, it makes it much easier to make changes later on if you can understand the intent behind the rules.

>Index: browser/components/preferences/handlers.xml

>+  <binding id="handler-base" extends="chrome://global/content/bindings/richlistbox.xml#richlistitem">
>+    <implementation>
>+      <property name="type">

readonly="true"

>+  <binding id="handler-selected" extends="chrome://browser/content/preferences/handlers.xml#handler-base">

>+            <xul:menupopup>
>+            </xul:menupopup>

<xul:menupopup/> ?

>Index: browser/components/preferences/content.xul

>-      <!-- FILE TYPES -->
>-      <preference id="pref.downloads.disable_button.edit_actions"
>-                  name="pref.downloads.disable_button.edit_actions"
>-                  type="bool"/>

I wonder if there's still interest in an easy way to disable this functionality? Perhaps worth a followup bug? mkaply might have an opinion.

>Index: browser/themes/winstripe/browser/preferences/preferences.css

>-/* bottom-most box containing a groupbox in a prefpane. Prevents the bottom
>-   of the groupbox from being cutoff */
>-.bottomBox {
>-  padding-bottom: 4px;
>-}

This looks like a bad merge, accidentally removing part of the patch from bug 394299.

r=me with those addressed.

Comment 48

[Alex Faaborg [:faaborg] (Firefox UX)]

>> * We should not provide a way to remove entries from the list, so remove the
>>   "Remove this Entry..." item from the drop-down menu of actions.
>
>I'm not sure why, but I'm ok with this for now.

I couldn't understand why that was there to begin with, what is the use case for later removing items in the list of available applications?

>> * We should sort so that the Web Feed item is on top followed by the rest of
>>   the entries in alphabetical order by type name.
>
>Why not mailto and others?  Kinda hard for me to justify feeds getting prime
>placement except to aid transitioning users...

All of the groups will be on top, once we have them implemented (like calendar = .ics, webcal://, hCalendar).  Feeds may eventually become a group if we start to tell the difference between a blog, podcast and video cast.  I agree that we should move mailto: into the top section of commonly used applications.

>> * For types you can save to disk, the "Save to Disk" item should appear at the
>>   end of the list, after "Choose Application...".
>
>Hmm, I'm not sure that's right.  I save things to disk quite a lot. :)

we need a way to support undo if you select "save to disk" in the content handling dialog and check "always do this."  Save to disk items should be listed after items that are associated with a particular application.

>> * We should unhide the "always ask" action and let the user decide to "always
>>   ask" about a type.
>
>What types do we have that will be set to always ask?

In the future all of the content in the groups (calendar, map, address book).  We might want to consider this for mailto: as well.

Myk: text should just be "Ask"

Comment 49

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v14: fixes issues brought up in testing and review

(In reply to comment #40)
> I've attempted V12 version of the patch.  I realize this is a work in progress,
> but here are my observations:
> * The application pane flickers spastically as it loads all of the items into
> itself

I have fixed this by setting an explicit height on the richlistbox listing the types.  I'm not sure this is the ideal solution, but it solves the problem temporarily at least.


> * When it is sitting with an empty search bar, the scroll bar only scrolls
> through the "a" entries, not all entries

This too should be fixed by the explicit height.


> * The oversized pref pane that is generated extends off my screen and cannot be
> resized (I'm running on a mac) Screen Shot 4.

This too should be fixed by the explicit height.


> * If I search for a specific protocol, then the protocol is listed.  I can
> select the default handler, but I cannot re-set the handler to "ask me what to
> do about this"

That's intentional, because the old code did this, and I wanted to keep things as similar as possible to reduce the regression surface, but I've filed bug 395143 on changing this, because I agree that it's the wrong thing to do.


> * If I search for a specific protocol, then the protocol is listed, and there
> are two buttons or headings or something at the bottom of the text window. 
> They are poorly clipped and I can't see what they are. Screen Shot 1.

Those are column headers.  They appear as footers because of bug 391740.  I'm not sure why they are cut off, but I think we should wait for bug 391740 to be fixed and then, if they are still cut off when they appear at the top of the list, file another bug on that issue.


> * When an entry is selected, the application goes to a white text on light gray
> background.  I have decent eyes and it's really hard to read. Screen Shot 2.

I think this is a Mac widget bug, but I'm not sure.  I'm going to talk to cbarrett about this and get his input, then file bug(s) as necessary.


> * General Usability (probably should be follow on bug): I don't think that a
> non-technical user is ever going to be able to use this, because they need to
> know the protocol name in order to change or modify the behavior.  Very few
> people know what a protocol is, and fewer will know that they must find
> "webcal" to change how "online calendars" are handled.  Of course, you do map
> "calendar" to ICS file, so that is a step in the right direction.

Right.  faaborg says we should hard-code mappings for popular protocols until OSes catch up and start providing this information.


> * Clicking off the "Applications" panel to some other pref panel also causes
> the spastic flickering.  

We can't avoid spastic flickering entirely, as Mac OS seems to think it's a good idea to resize the dialog every time you switch prefpanes, but it should be less spastic now that the richlistbox has an explicit height.


> * Click on the applications panel, search for something, switch to a different
> preference pane, click back on the applications panel. Now, the applications
> pane remains its previous size (rather small).  The list of items flows through
> the dialog, and is not scrollable and does not have an end. Screen shot 3.

This too should be fixed by the explicit height.


Note: I also noticed that all prefpanes were a bit cut off along their bottom edge.  I traced that back to flex on the Applications prefpane.  Removing the flex fixed the problem.


(In reply to comment #47)
> global nit: I saw "switch(" in a few places, please add a space?

Yup, added.


> You could use an expression closure here:
> disabledPluginTypes = disabledPluginTypes.filter(function(v) v != type);

Good idea.  That makes the statement short enough to fit on one line.



> >+  get smallIcon() {
> >+  get largeIcon() {
> 
> Perhaps worth factoring out the common code for these two?

Yup, I have done so.


> This could just be me misunderstanding how this code works, but don't we want
> to always add the default handler, should the user want to choose it? Ah, I
> guess the code in rebuildActionsMenu takes care of that, and
> possibleApplicationHandlers is only supposed to include non-default handlers?

Right.  In the long term the default handler should be one of the items in possible handlers, but at the moment the backend distinguishes between "use default" and "use helper app", and it doesn't provide with an nsIHandlerApp representing the default handler, just a "defaultDescription" string (and the hasDefaultHandler boolean indicating whether or not there is a default handler), so we add the default handler to the list of possible handlers separately.


> >+  // XXX Shouldn't we memoize this value so we don't have to keep retrieving it
> >+  // over and over again?
> >+  get _defaultApplicationHandler() {
> 
> Sounds like a good idea, followup bug? :)

This one was actually a simple fix, so I've added it to this patch.


> >+  // What to do with content of this type.
> >+  get preferredAction() {
> 
> >+      // If the action is "ask", then alwaysAskBeforeHandling will override
> >+      // the action, so it doesn't matter what we say it is, it just has to be
> >+      // something that doesn't cause the controller to hide the type.
> >+      case "ask":
> >+      default:
> >+        Ci.nsIHandlerInfo.handleInternally;
> 
> Missing return?

Good catch.  Fixed.


> >+  _getVisibleTypes: function() {
> 
> >+      // XXX If the type has a plugin, should we also check the "suffixes"
> >+      // property of the plugin?
> 
> >+      // XXX "handledOnlyByPlugin" doesn't mean there isn't an OS-default
> >+      // handler for the type, it only means there isn't a user-configured
> >+      // handler.  Don't we want to give the user the option to switch to
> >+      // the OS-default handler for these types?
> 
> >+      // XXX Shouldn't we show these types to give the user a chance
> >+      // to reenable the plugins?
> 
> Worth filing a followup bug for these?

Yup, the first one is bug 395135.

After further thought, I think the second issue is bogus.  We show plugins by default, but if someone twiddles the pref and hides them, the probably don't want to see them, and we shouldn't show them.

The third one is bug 395136.


> >+      // Don't display entries for types whose preferred action is alwaysAsk.
> >+      // alwaysAsk is actually an obsolete preferred action (we now keep track
> >+      // of that state separately via the alwaysAskBeforeHandling flag),
> >+      // and we shouldn't see it anymore, and it's not clear why we're checking
> >+      // for it here, but the old code did that, so for now we're doing it too.
> >+      if (handlerInfo.preferredAction == Ci.nsIHandlerInfo.alwaysAsk)
> >+        continue;
> 
> This sounds like it would lead to bugs where people can't change the action for
> a download without hand-editing mimeTypes.rdf. Is that not the case?

I went back and re-read the code, and it turns out that the old code actually looked at NC:alwaysAsk in the datasource, which corresponds to the nsIHandlerInfo::alwaysAskBeforeHandling flag, not the obsolete nsIHandlerInfo::alwaysAsk action.

So I've changed this conditional to look at the flag instead.  This does mean that types set to "always ask" are hidden in the prefpane just as they were hidden in the old Download Actions dialog, which was always my intention, since it reduces the regression surface to do the things the old interface did.

Nevertheless, this doesn't make a whole lot of sense to me, since it's likely to be confusing to users who expect to be able to see all the types they've ever downloaded here (not just the ones they had the foresight to turn off the "always ask" flag for).  But changing that is a larger change than I think we can do at this point in this bug, so I've spun off bug 395138 on this.


> >+        // For other types, handleInternally looks like either useHelperApp
> >+        // or useSystemDefault depending on whether or not there's a preferred
> >+        // handler app.
> 
> >+        // XXX Why don't we say the app will handle the type internally?
> >+        // Is it because the app can't actually do that?  But if that's true,
> >+        // then why would a preferredAction ever get set to this value
> >+        // in the first place?
> 
> What was the rationale behind the first comment, given the second?

The first comment is the way it currently works (which is the same as the way it used to work in the old code).  The second comment represents my puzzlement over why it works that way.


> >+  _sortTypes: function(aTypes) {
> >+    // FIXME: strict warning: function does not always return a value.
> 
> Just make it "return undefined;" ?

Hmm, I'm sorting the array inplace, so I actually don't need any return value.  I've removed both "return" statements.


> >+  onFilterKeyPress: function(aEvent) {
> >+    if (aEvent.keyCode == 27) // ESC key
> 
> Use KeyEvent.DOM_VK_ESCAPE

Yup, changed.


> >+  onSelectAction: function(event) {
> 
> >+      // XXX Is there any value in leaving the existing preferred app,
> >+      // if any, as it is when we set the preferred action to something
> >+      // other than useHelperApp?
> 
> Perhaps worth a followup (noted in the XXX comment)? If this leaves the
> existing preferred app in the list, I think we should leave it.

The preferred app will stay in the list for new datastores, but it won't stay there for old ones, so I've filed a bug to fix this.  I'd have done it immediately, but I need to think through the implications first.  There may be some gotchas to doing so.


> >+  chooseApp: function(aEvent) {
> 
> >+      // XXX Should we not stop propagation above and then not store here,
> >+      // relying on the normal handler to store?
> >+      handlerInfo.store();
> 
> I'd remove this XXX and leave things as-is, I think it's cleaner and easier to
> understand that way.

Ok, done.


> >+  _getIconURLForWebApp: function(aWebAppURL) {
> 
> Could we use the faviconservice for this?

Unfortunately not, since the favicon service only retrieves favicon matches by exact URL, not domain!  And these exact URLs won't be in the database, since users don't go to them, they go to the version of the URL that has had the %s replaced by some URL.

But I've added a comment to this effect.


> >+  _getIconURLForSystemDefault: function(aHandlerInfo) {
> 
> >+    if (aHandlerInfo instanceof Ci.nsIMIMEInfo &&
> >+        aHandlerInfo instanceof Ci.nsIPropertyBag) {
> >+      try {
> >+        var url = aHandlerInfo.getProperty("defaultApplicationIconURL");
> >+        if (iconURL)
> >+          return iconURL + "?size=16";
> 
> Looks like you meant to use "url" here? Was this not tested?

This wasn't tested yet, since I had only tested on Linux and Mac before attaching the patch, and this code only works on Windows.  Fixed (and tested and verified that it works).


> >+  <binding id="handler-base" extends="chrome://global/content/bindings/richlistbox.xml#richlistitem">
> >+    <implementation>
> >+      <property name="type">
> 
> readonly="true"

Yup, added.


> >+  <binding id="handler-selected" extends="chrome://browser/content/preferences/handlers.xml#handler-base">
> 
> >+            <xul:menupopup>
> >+            </xul:menupopup>
> 
> <xul:menupopup/> ?

It had some content in it at one point, but none now.  Shortened.


> >-      <!-- FILE TYPES -->
> >-      <preference id="pref.downloads.disable_button.edit_actions"
> >-                  name="pref.downloads.disable_button.edit_actions"
> >-                  type="bool"/>
> 
> I wonder if there's still interest in an easy way to disable this
> functionality? Perhaps worth a followup bug? mkaply might have an opinion.

Yup, I'll ping him and ask about it.


> >Index: browser/themes/winstripe/browser/preferences/preferences.css
> 
> >-/* bottom-most box containing a groupbox in a prefpane. Prevents the bottom
> >-   of the groupbox from being cutoff */
> >-.bottomBox {
> >-  padding-bottom: 4px;
> >-}
> 
> This looks like a bad merge, accidentally removing part of the patch from bug
> 394299.

Yup, I've added it back.


> r=me with those addressed.

Here's the version with those issues addressed.  Carrying forward your review.  This is the version of the patch I'll check in.

Note: I've added bug numbers to all FIXME comments in applications.js.  But those are a subset of all the bugs I've filed, which are themselves a subset of all the bugs I'm going to file (f.e. most of faaborg's nits have not made their way to bugs yet).

Comment 50

[Myk Melez [:myk] [@mykmelez]]

Checking in browser/components/preferences/preferences.xul;
/cvsroot/mozilla/browser/components/preferences/preferences.xul,v  <--  preferences.xul
new revision: 1.13; previous revision: 1.12
done
Checking in browser/components/preferences/jar.mn;
/cvsroot/mozilla/browser/components/preferences/jar.mn,v  <--  jar.mn
new revision: 1.19; previous revision: 1.18
done
RCS file: /cvsroot/mozilla/browser/components/preferences/applications.xul,v
done
Checking in browser/components/preferences/applications.xul;
/cvsroot/mozilla/browser/components/preferences/applications.xul,v  <--  applications.xul
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/browser/components/preferences/applications.js,v
done
Checking in browser/components/preferences/applications.js;
/cvsroot/mozilla/browser/components/preferences/applications.js,v  <--  applications.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/browser/components/preferences/handlers.css,v
done
Checking in browser/components/preferences/handlers.css;
/cvsroot/mozilla/browser/components/preferences/handlers.css,v  <--  handlers.css
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/browser/components/preferences/handlers.xml,v
done
Checking in browser/components/preferences/handlers.xml;
/cvsroot/mozilla/browser/components/preferences/handlers.xml,v  <--  handlers.xml
initial revision: 1.1
done
Checking in browser/components/preferences/content.xul;
/cvsroot/mozilla/browser/components/preferences/content.xul,v  <--  content.xul
new revision: 1.22; previous revision: 1.21
done
Checking in browser/components/preferences/content.js;
/cvsroot/mozilla/browser/components/preferences/content.js,v  <--  content.js
new revision: 1.15; previous revision: 1.14
done
Removing browser/components/preferences/changeaction.js;
/cvsroot/mozilla/browser/components/preferences/changeaction.js,v  <--  changeaction.js
new revision: delete; previous revision: 1.4
done
Removing browser/components/preferences/changeaction.xul;
/cvsroot/mozilla/browser/components/preferences/changeaction.xul,v  <--  changeaction.xul
new revision: delete; previous revision: 1.10
done
Removing browser/components/preferences/downloadactions.js;
/cvsroot/mozilla/browser/components/preferences/downloadactions.js,v  <--  downloadactions.js
new revision: delete; previous revision: 1.12
done
Removing browser/components/preferences/downloadactions.xul;
/cvsroot/mozilla/browser/components/preferences/downloadactions.xul,v  <--  downloadactions.xul
new revision: delete; previous revision: 1.9
done
Removing browser/components/preferences/feeds.js;
/cvsroot/mozilla/browser/components/preferences/feeds.js,v  <--  feeds.js
new revision: delete; previous revision: 1.10
done
Removing browser/components/preferences/feeds.xul;
/cvsroot/mozilla/browser/components/preferences/feeds.xul,v  <--  feeds.xul
new revision: delete; previous revision: 1.6
done
Checking in browser/themes/winstripe/browser/preferences/preferences.css;
/cvsroot/mozilla/browser/themes/winstripe/browser/preferences/preferences.css,v  <--  preferences.css
new revision: 1.20; previous revision: 1.19
done
Checking in browser/themes/pinstripe/browser/preferences/preferences.css;
/cvsroot/mozilla/browser/themes/pinstripe/browser/preferences/preferences.css,v  <--  preferences.css
new revision: 1.21; previous revision: 1.20
done
Checking in browser/locales/jar.mn;
/cvsroot/mozilla/browser/locales/jar.mn,v  <--  jar.mn
new revision: 1.71; previous revision: 1.70
done
RCS file: /cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/applications.dtd,v
done
Checking in browser/locales/en-US/chrome/browser/preferences/applications.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/applications.dtd,v  <--  applications.dtd
initial revision: 1.1
done
Checking in browser/locales/en-US/chrome/browser/preferences/preferences.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/preferences.dtd,v  <--  preferences.dtd
new revision: 1.10; previous revision: 1.9
done
Checking in browser/locales/en-US/chrome/browser/preferences/preferences.properties;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/preferences.properties,v  <--  preferences.properties
new revision: 1.14; previous revision: 1.13
done
Checking in browser/locales/en-US/chrome/browser/preferences/content.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/content.dtd,v  <--  content.dtd
new revision: 1.13; previous revision: 1.12
done
Removing browser/locales/en-US/chrome/browser/preferences/changeaction.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/changeaction.dtd,v  <--  changeaction.dtd
new revision: delete; previous revision: 1.3
done
Removing browser/locales/en-US/chrome/browser/preferences/downloadactions.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/downloadactions.dtd,v  <--  downloadactions.dtd
new revision: delete; previous revision: 1.4
done
Removing browser/locales/en-US/chrome/browser/preferences/feeds.dtd;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/feeds.dtd,v  <--  feeds.dtd
new revision: delete; previous revision: 1.2
done
Removing browser/locales/en-US/chrome/browser/preferences/feeds.properties;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/feeds.properties,v  <--  feeds.properties
new revision: delete; previous revision: 1.2
done
Checking in browser/components/feeds/public/nsIWebContentConverterRegistrar.idl;
/cvsroot/mozilla/browser/components/feeds/public/nsIWebContentConverterRegistrar.idl,v  <--  nsIWebContentConverterRegistrar.idl
new revision: 1.6; previous revision: 1.5
done
Checking in browser/components/feeds/src/WebContentConverter.js;
/cvsroot/mozilla/browser/components/feeds/src/WebContentConverter.js,v  <--  WebContentConverter.js
new revision: 1.22; previous revision: 1.21
done
Checking in toolkit/mozapps/jar.mn;
/cvsroot/mozilla/toolkit/mozapps/jar.mn,v  <--  jar.mn
new revision: 1.37; previous revision: 1.36
done
Removing toolkit/mozapps/preferences/actionsshared.js;
/cvsroot/mozilla/toolkit/mozapps/preferences/actionsshared.js,v  <--  actionsshared.js
new revision: delete; previous revision: 1.2
done

Comment 51

[pal-moz]

with Windows,
[Content Type][Application] should be placed at the top, not bottom ?

see below,
http://forums.mozillazine.org/viewtopic.php?p=3044155#3044155
http://forums.mozillazine.org/viewtopic.php?p=3044204#3044204

Comment 52

[Elmar Ludwig]

(In reply to comment #51)
> with Windows,
> [Content Type][Application] should be placed at the top, not bottom ?

That's bug 391740 (see comment #49 above).

Comment 53

[pal-moz]

thank you for the info.

Comment 54

[AndrewM]

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a8pre) Gecko/2007091005 Minefield/3.0a8pre ID:2007091005

With a clean profile and no extensions, the "Save to Disk" option isn't appearing in the drop-down menu for any content type for me. Should it be appearing in the current nightly?

Comment 55

[Myk Melez [:myk] [@mykmelez]]

> With a clean profile and no extensions, the "Save to Disk" option isn't
> appearing in the drop-down menu for any content type for me. Should it be
> appearing in the current nightly?

It should be, yes.  I have filed bug 395713 on the issue and cc:ed you on that bug.

Comment 56

[Myk Melez [:myk] [@mykmelez]]

Correcting dependency tree to show fallout from this bug (regressions, polish, cleanup, etc.) as dependencies on this bug rather than blockers.

Note that bug 395135 and bug 395645 are not listed as dependencies, since they don't actually depend on this bug (they could have been fixed independently) and are unlikely to be fixed in the timeframe available for fixing fallout from this bug.

Comment 57

[Myk Melez [:myk] [@mykmelez]]

Hrm, apparently regressions are supposed to block the bug that caused the regression, even though it seems nonsensical to have blockers for resolved bugs, and it means splitting up followup work between blockers and dependencies.

But that's the convention, so moving regression bugs back to the blockers list.

Comment 58

[Alex Faaborg [:faaborg] (Firefox UX)]

to remain consistent with bug #301972, we need to change the implementation centric phrase "Save to Disk" to phrase "Save File"

Comment 59

[Carsten Book [:Tomcat]]

verified fixed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8) Gecko/2007091216 GranParadiso/3.0a8 (Gran Paradiso M8 RC1) on Linux/Windows/Mac

I still see UI problems with the Content Type/Application bar in this build, but the UI itself works.

- > verified

Comment 60

[Marcia Knous [:marcia]]

Litmus Triage Team: ctalbert, will you cover the Litmus test case for this one?

Comment 61

[cmtalbert]

(In reply to comment #60)
> Litmus Triage Team: ctalbert, will you cover the Litmus test case for this one?
> 
yes

Comment 62

[cmtalbert]

We have litmus cases for this:
https://litmus.mozilla.org/show_test.cgi?searchType=by_category&product_id=1&branch_id=15&testgroup_id=55&subgroup_id=884
And
https://litmus.mozilla.org/show_test.cgi?searchType=by_category&product_id=1&branch_id=15&testgroup_id=56&subgroup_id=820

Comment 63

[Carl Ponder]

Currently the list of actions is determined bt the "content type", which depends on FireFox being able to figure out the type of the file. There should also be a mechanism to associate actions based on the filename extension, for example

text     *.csv      libreoffice4.1
text     *          vim

This would help in the many cases where the FireFox design does not outguess the user.