548426 - xpcom/tests/TestFile.exe fails for windows mobile

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

Three of the tests within this test fail.  
* Path Normalization fails because it the implementation for Normalize() was ifdef'd out. 
* Appending an invalid string to a path failed because the check for that for that was ifndef'd out. 
* Setting the DELETE_ON_CLOSE flag failed because windows ce seems to not have this concept.

This patch un-ifndef's the two bits sections of code that causes failures and punts on the DELETE_ON_CLOSE test. Any suggestions for a better fix for DELETE_ON_CLOSE are more than welcome.

I also removed some other seemingly extraneous ifdnefs.  Figured those changes could come along for the ride, but I can split them out for the purposes of review if you want.

Comment 1

[Benjamin Smedberg]

Comment on attachment 428814 [details] [diff] [review]
patch

I'm fine with the changes overall, except that if WinCE really doesn't have any DELETE_ON_CLOSE ability, OpenNSPRFileDesc should throw an exception when you pass it, instead of silently leaving the file around.

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch v.2

Comment 3

[Benjamin Smedberg]

Comment on attachment 431376 [details] [diff] [review]
patch v.2

You'll need to change the test, or just ifdef out the part that tests delete-on-close.

Comment 4

[u60234]

Comment on attachment 431376 [details] [diff] [review]
patch v.2

>--- a/xpcom/io/nsLocalFileWin.cpp
>+++ b/xpcom/io/nsLocalFileWin.cpp

> NS_IMETHODIMP
> nsLocalFile::Reveal()
> {
>-#ifndef WINCE
>     // make sure mResolvedPath is set
>     nsresult rv = ResolveAndStat();
>     if (NS_FAILED(rv) && rv != NS_ERROR_FILE_NOT_FOUND)
>         return rv;
>     
>     // use the full path to explorer for security
>     nsCOMPtr<nsILocalFile> winDir;
>     rv = GetSpecialSystemDirectory(Win_WindowsDirectory, getter_AddRefs(winDir));
>     NS_ENSURE_SUCCESS(rv, rv);
>     nsAutoString explorerPath;
>     rv = winDir->GetPath(explorerPath);  
>     NS_ENSURE_SUCCESS(rv, rv);
>+#ifdef WINCE
>     explorerPath.Append(L"\\explorer.exe");
>-    
>+#else
>+    explorerPath.Append(L"\\fexplorer.exe");
>+#endif

There is a typo here, "fexplorer.exe". It breaks the "Open Containing Folder" button in about:support in Minefield.

Comment 5

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

Comment on attachment 431376 [details] [diff] [review]
patch v.2

># HG changeset patch
># User Brad Lassey <[e-mail]>
># Date 1268151869 18000
># Node ID 60dd175f8012d9bdc83c31e0425422542aa25266
># Parent  5a35586a968853a978c94fd4d44c73e669634b1d
>[mq]: xpcom-file-test
>
>diff --git a/xpcom/io/nsLocalFileWin.cpp b/xpcom/io/nsLocalFileWin.cpp
>--- a/xpcom/io/nsLocalFileWin.cpp
>+++ b/xpcom/io/nsLocalFileWin.cpp
>...
> NS_IMETHODIMP
> nsLocalFile::Reveal()
> {
>-#ifndef WINCE
>     // make sure mResolvedPath is set
>     nsresult rv = ResolveAndStat();
>     if (NS_FAILED(rv) && rv != NS_ERROR_FILE_NOT_FOUND)
>         return rv;
>     
>     // use the full path to explorer for security
>     nsCOMPtr<nsILocalFile> winDir;
>     rv = GetSpecialSystemDirectory(Win_WindowsDirectory, getter_AddRefs(winDir));
>     NS_ENSURE_SUCCESS(rv, rv);
>     nsAutoString explorerPath;
>     rv = winDir->GetPath(explorerPath);  
>     NS_ENSURE_SUCCESS(rv, rv);
>+#ifdef WINCE
>     explorerPath.Append(L"\\explorer.exe");
>-    
>+#else
>+    explorerPath.Append(L"\\fexplorer.exe");
>+#endif
Hey Brad, this looks backwards and is the cause of bug 552602.

Comment 6

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

And it got pushed on March 9th.
http://hg.mozilla.org/mozilla-central/rev/9e551fadad8d

Comment 7

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

or if it is a typo as Hasse suggests the ifdef's should just be removed

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

that certainly  looks backwards. I'm not sure I'll be able to watch the tree long enough to push a fix for a few days. Rob, would you mind?

Comment 9

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

Not a problem... I'll take care of it during the day tomorrow.

Comment 10

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

It landed yesterday. Not sure if this bug should be left open since the patch has landed on trunk.

Comment 11

[Ed Morley [:emorley]]

WinCE/Windows Mobile support has been removed from the main build system, Spidermonkey, mobile installer, in-app updater and so on (see bug 614720, bug 554087 and all their dependants). Until such point where MS decide to release a Windows Phone 7 NDK and the decision is made to port to that platform, this is WONTFIX.

Filter bugmail on WinCEMassWONTFIX.