Opened 6 months ago

Closed 5 months ago

Last modified 5 months ago

#16005 closed defect (fixed)

wxDirDialog::GetPath() returns erroneous results on Windows Vista

Reported by: jtrauntvein Owned by:
Priority: low Milestone:
Component: wxMSW Version: 3.0.0
Keywords: wxDirDialog Windows Vista Cc:
Blocked By: Blocking:
Patch: yes

Description

If my application uses an instance of wxDirDialog to allow the user to browse to a directory, on Windows Vista, the value returned from GetPath() can sometimes include the last path component twice depending upon how the user interacted with the directory dialogue. I have found that I can work around this by disabling wxUSE_IFILEDIALOG.

Attachments (1)

patch download (452 bytes) - added by jtrauntvein 6 months ago.
Patch that prevents the use of the IFILEDIALOG for Vista

Download all attachments as: .zip

Change History (11)

Changed 6 months ago by jtrauntvein

Patch that prevents the use of the IFILEDIALOG for Vista

comment:1 Changed 6 months ago by vadz

Has anybody else observed this misbehaviour under Vista? I don't care much about it one way or the other, so I guess we should turn the new style dialog off for it just to be on the safe side but it seems incredibly suspicious that something as basic as this wouldn't work.

Have you tried reporting this as a bug to Microsoft by chance?

comment:2 Changed 6 months ago by PB

(I wrote most of the wxDirDialog code using IFileDialog).

Unfortunately, I don't have Windows Vista to test, but here's what I googled out:
http://support.microsoft.com/kb/969885/en-us

The bug description looks very similar to what is described in the ticket, the bug is triggered by selecting a folder from the tree view and not the folder view.

comment:3 Changed 6 months ago by jtrauntvein

I read the knowledge base article referenced by PB above and the description matches the behaviour that I have seen exactly.

comment:4 Changed 6 months ago by PB

I guess adding a workaround to wxWidgets allowing to use the new style dialog on Vista is not a wxWidgets' way?

I.e.

  1. Obtain the path from IDialog as usual.
  2. If on Vista/Server 2008 check if the path is valid. If it is not, remove its last component and check again. If this path is valid, return it instead of the original path from IFileDialog.

Possible issues with this solution:

  1. Adds (a bit of a) bloat to wxWidgets.
  2. If a path is remote, its validating may be expensive.
  3. The path the user selected was valid when he did it, but its deepest folder was deleted in those few milliseconds before we checked its validity. Very unlikely, but still theoretically possible.

comment:5 follow-up: Changed 6 months ago by vadz

  • Priority changed from normal to low
  • Status changed from new to confirmed

Thanks for finding the support article!

I don't like the idea of "auto-correcting" the path because of the last issue with it (which got numbered "2" above but should have been "3" presumably): we can return a wrong path if we do this. And it's not that unlikely to have something like "c:\src\src".

So we should either:

  1. Do nothing and tell people affected by this to install a hotfix.
  2. Apply this patch and disable the use of this dialog entirely under Vista.
  3. Test for the exact DLL version (support article has the details) and only use this dialog if it's high enough.

I guess the latter would be the best but personally I think (1) is fine too...

comment:6 in reply to: ↑ 5 ; follow-up: Changed 6 months ago by PB

Replying to vadz:

Thanks for finding the support article!

I don't like the idea of "auto-correcting" the path because of the last issue with it (which got numbered "2" above but should have been "3" presumably): we can return a wrong path if we do this. And it's not that unlikely to have something like "c:\src\src".

Sorry, I mistyped the algorithm for auto-correcting the path, it should read (only on Vista):

  1. Check if there are at least two folders in the path. If not, go to 6.
  2. Check if the last two folder names are same. If not, go to 6.
  3. Check if the path is valid. If it is, go to 6.
  4. Remove the last folder from the path and validate it again.
  5. If the adjusted path is valid, change the path to the adjusted one.
  6. Return the path.

But I do agree this is still not 100% error-prone, see the hypothetical scenario involving deleting the deepest folder in my previous post.

On the other hand, I can not agree with your solution 1. This bug directly affects the application users, not application programmers. Users (and many devs) don't read developer documentation nor they generally (can) install hotfixes. This leaves us with solutions 2 and 3, with 2 being easiest to implement but leaving the Vista user base with that old unfriendly SHBrowseForFolder() dialog and I believe such users have already suffered enough with Vista...

comment:7 in reply to: ↑ 6 Changed 5 months ago by pwray

I confirm this bug.

comment:8 Changed 5 months ago by vadz

OK, let's disable the use of the new dialog in 3.0.1. If anybody else can be bothered to make a patch testing for the exact DLL version (and test it under Vista systems with and without the hotfix, which is the time-consuming part), I'd be glad to apply it later.

comment:9 Changed 5 months ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [76151]) Disable the use of new style wxDirDialog under Vista.

IFileDialog-based dialog has a bug making it return a wrong path sometimes
under Vista, disable its use there to avoid it.

Closes #16005.

comment:10 Changed 5 months ago by VZ

(In [76152]) Disable the use of new style wxDirDialog under Vista.

IFileDialog-based dialog has a bug making it return a wrong path sometimes
under Vista, disable its use there to avoid it.

Closes #16005.

Note: See TracTickets for help on using tickets.