340576 - File icons not displayed in filepicker in firefox classic skin

Status: UNCONFIRMED

(Toolkit :: UI Widgets, defect)

Description

[Luke Sandell]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.0.4) Gecko/20060604 Firefox/1.5.0.4
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.0.4) Gecko/20060604 Firefox/1.5.0.4

I know that filepicker is pretty much deprecated, but I reenabled it because I intend to use it in my Mozilla-based file manager I am created. However, there is a problem with the Firefox Classic skin that keeps it from displaying icons next to files. See fix in Additional Information.

I didn't see a category for the default skin, so I am posting here.

Reproducible: Always

Steps to Reproduce:
1. Make sure you are using the "Classic" skin.
2. Enable the built-in filepicker as described at http://forums.mozillazine.org/viewtopic.php?t=318860&highlight=filepicker
3. Restart firefox.
4. Do File->Open File
5. Change the filter to "All Files (*)" in order to display some files.

Actual Results:  
Regular files are shown without icons.

Expected Results:  
Regular files should be shown with the generic "document" icon.

The culprit is a line in filepicker.css:

treechildren::-moz-tree-image(FilenameColumn, file) {
  list-style-image: url("chrome://global/skin/icons/folder-item.png");
  -moz-image-region: rect(16px, 16px, 32px, 0px);
}

SHOULD BE AS FOLLOWS:

treechildren::-moz-tree-image(FilenameColumn, file) {
  list-style-image: url("chrome://global/skin/icons/folder-item.png");
  -moz-image-region: rect(0px, 16px, 16px, 0px);
}

Maybe this was done on purpose, because somebody didn't want the icons? I noticed Mozilla's skin still has them.

Just an aesthetic nitpick. P.S. -- if you need a quasi-maintainer for nsFilePicker/nsFileView, I am making and have made several improvements to nsFileView, such as the addition of new columns. It would be nice to get these into the main code base so that I do not have to distribute a binary version of the C++ class with my application, and I would be glad to send a patch :-)

Comment 1

[Phil Ringnalda (:philor)]

A |cvs diff -up8|, added with the "Create a New Attachment" link, with first-review set to "?" and "[e-mail]", would be a great start.

Comment 2

[Luke Sandell]

(In reply to comment #1)
> A |cvs diff -up8|, added with the "Create a New Attachment" link, with
> first-review set to "?" and "[e-mail]", would be a great start.
> 

You mean for any other improvements I happen to make? Surely you don't need a patch for the above fix, as it only involves changing two numbers. Further, my other modifications are in nsFileView.cpp and have nothing to do with the style.

Comment 3

[Phil Ringnalda (:philor)]

Yup. Other than some rubber-stamp exceptions like spelling errors in comments, nothing goes in without a bug, a patch, and review. http://www.mozilla.org/hacking/ though it probably won't lead you to http://www.mozilla.org/projects/toolkit/review.html which is the review requirements for this module.