136941 - filters from filepicker.properties should should have case insensitive test

Status: RESOLVED FIXED

(Core :: Widget: Gtk, enhancement)

Description

[Geraint]

Trying to open a file "XXX.JPG" using the "filepicker" and the image filter
fails and I cannot see the file, but if I add "*.JPG" to the imageFilter field
in filepicker.properties I do see the file.  

The file extensions should be compared on a case insensitive basis or both the
lower case and upper case extensions should be added as file filters to
filepicker.properties?

Comment 1

[Kai Lahmann (is there, where MNG is)]

what's the status of this?

Comment 2

[timeless]

xpfilepicker

Comment 3

[Kenneth Herron]

I'm going to confirm this as an enhancement request. It seems reasonable enough.

Comment 7

[Phil Ringnalda (:philor)]

What with all the dupes being on Linux, and even the XUL filepicker on Linux working fine, this would seem to be a Widget: GTK bug.

Comment 8

[Michael Ventnor]

Attachment: Patch

For some reason I'm getting the feeling that this approach is a little dangerous, but so far it works. GTK takes a shell blob for its pattern matching which is like regular expressions ultra lite. It seems to me from documentation /*/i wouldn't work.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Looks OK, but g_strstrip needs its result to be freed, so this is actually leaking (and always was).

+MakeCaseInsensitiveShellGlob(const char* existingString) {

aExistingString

You should also add a comment that aExistingString is a UTF8 string, but that this function is OK because g_ascii_isalpha will return false for any non-ASCII character in the string so all non-ASCII characters are just passed through.

Comment 10

[Michael Ventnor]

Attachment: Patch 2

g_strstrip is a macro which doesn't need freeing (as I've explained IRL). Added the comment.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #9)
> +MakeCaseInsensitiveShellGlob(const char* existingString) {
> 
> aExistingString

You didn't address this comment.

Please mention in your comment that aExistingString is UTF8.

Comment 12

[Michael Ventnor]

Attachment: Patch 3

Whoops, sorry, I missed that comment.

Comment 13

[Michael Ventnor]

Comment on attachment 349684 [details] [diff] [review]
Patch 3

Since this old bug has a fairly trivial fix (ignore my first comment, I now know what shell globs are/aren't capable of) it would be nice to get this out for 1.9.1.

Comment 14

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

Comment on attachment 349684 [details] [diff] [review]
Patch 3

a191=beltzner - should we add a test for this new test?

Comment 15

[Michael Ventnor]

I don't think you can, really. This bug is all about a modal dialog for an external toolkit. The best you can get is a Litmus test, I suppose.

Comment 16

[Michael Ventnor]

Attachment: Patch with commit message

For checkin

Comment 17

[Shawn Wilsher :sdwilsh]

http://hg.mozilla.org/mozilla-central/rev/40c71c199ff7