236227 - crash if requesting drive icon with moz-icon:

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Philippe GOETZ]

User-Agent:       
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20040219

The moz-icon: library crashes when requesting the icon of a drive
moz-icon:file:///C|/. This is true with all drives (at least A: and C:).

Reproducible: Always
Steps to Reproduce:
1. Enter the url moz-icon:file:///C|/
Actual Results:  
Crash

Expected Results:  
Returning the icon of the drive (amovible, fixed, cd-rom, etc...) - expected to
be found in shell32.dll

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

might've been fixed by bug 235537; the crash part at least, although it won't
make the icon show

Comment 2

[[e-mail]]

The icon doesn't show because line 748 (or so) of nsLocalFileWin.cpp strips the
\ from C:\ so that you get C: instead; unfortunately you can't just not strip
the \ because stuff like AppendNative expects it to be stripped.

Comment 3

[Philippe GOETZ]

In order to retrieve the icon related to the drives make the following patch:

in mozilla/modules/libpr0n/decoders/icon/win/nsIconChannel.cpp

Add the following lines in function nsIconChannel::MakeInputStream 

  if(filePath.Last()==PRUnichar(':')) {
    filePath.Append(PRUnichar('\\'));
  }

after

  if (!fileExists)
      infoFlags |= SHGFI_USEFILEATTRIBUTES;

Comment 4

[Philippe GOETZ]

A better patch for the icon of drives (which avoid to be asked to insert the
drive ):

File: mozilla/modules/libpr0n/decoders/icon/win/nsIconChannel.cpp
Function: nsIconChannel::MakeInputStream (line #220)
...
  if (localFile)
  {
    localFile->GetNativePath(filePath);
+    if(filePath.Last()==PRUnichar(':'))  
+      filePath.Append(PRUnichar('\\'));
+    else
      localFile->Exists(&fileExists);
  }
...

Comment 5

[[e-mail]]

Hmm... won't this last patch fail to check if the drive exists?

Comment 6

[Philippe GOETZ]

(In reply to comment #5)
> Hmm... won't this last patch fail to check if the drive exists?

This is not a problem with Windows (at least XP). Because Windows returns a
special icon indicating an indexistant drive (the drive icon with a white
question mark in a red ball). Try it (e.g. moz-icon:file:///Z|/ when Z: doesn't
exist).
This is very useful when you build a file browser in a tree with a template
using  rdf:files. The patch avoids to be asked for a disk for removable drives
e.g. A: (or a CD-ROM) when no disk is present. A pitty...

Consider that...

Comment 7

[[e-mail]]

Attachment: Proposed patch

OK, so this works for missing drives, but it doesn't seem to like to generate
icons for missing files, so I added in a few extra lines to handle those.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

does os/2 need a similar change?

what does nt 3.51 do? :)

Comment 9

[Mike Kaply [:mkaply]]

on os/2 I'm asked to download the icon, it doesn't display. I vaguely remember a
difference with the moz-icon protocol on OS/2....

Comment 10

[Philippe GOETZ]

(In reply to comment #8)
> does os/2 need a similar change?
> 
> what does nt 3.51 do? :)

Mozilla 1.7b would not install on Windowd NT 3.51 Server SP5!!!!
- missing/innacessible MSVCRT.DLL
+ many other request of the same style

Comment 11

[[e-mail]]

(In reply to comment #10)
>Mozilla 1.7b would not install on Windowd NT 3.51 Server SP5!!!!
That's an installer issue, most of 1.7 runs on NT 3.51

Comment 12

[Reed Loden [:reed]]

Neil,
Is this patch still relevant? I don't see this patch added in trunk, so I'm thinking it was never committed.

Comment 13

[[e-mail]]

Comment on attachment 145130 [details] [diff] [review]
Proposed patch

This just fixes the aspect that you can't get icons for drives from moz-icon:

Comment 14

[[e-mail]]

Drive icon fix checked in.