File type dialog in "Save Page As" dialog shows an invalid option or shows "Web page, complete" only. This bug is reproducable from 2002092404-trunk/Win98.

I can't reproduce this problem in the 2002-10-09-08 trunk build. Tested under Windows ME. Reporter, can you try with a newer trunk build ?

I can reproduce the problem in Win32 trunk build 2002111204.

This bug had gone once, since 2002-09-29. Now it returned after 2002-11-11-04-trunk/Win98SE.

I suspect patch of bug 104934 would cause this bug. It was checked in on 09/23/2002 12:16, backed out on 09/28/2002 08:52 and checked in again on 11/08/2002 14:46-14:47. This bug can be reproduced between 2002092404-trunk and 20020928??-trunk, and 2002111104-trunk or later. It cannot be seen from 2002092908-trunk to 2002110808-trunk.

reporter: is this Win98/ME only problem? Which language OS is this? assign bug to myself

I have only Japanese Windows 98 Second Edition. Therefore I can confirm this bug on it. Per Bugzilla-JP comments, this can be reproduced on Japanese Windows 98, not on Japanese Windows XP. There is no report about Japanese Windows Me. http://bugzilla.mozilla.gr.jp/show_bug.cgi?id=2696

Same build (2002112108) exhibits this problem on Win98, but not on Win2K, so it probably is Win9x only.

*** Bug 181608 has been marked as a duplicate of this bug. ***

I see this on Norwegian Win95 with trunk-build 2002111804. The 1.2 branch-build 2002111817 is not affected. Duplicate bug 181608 is reported with en-US win98.

Also noted in duplicate bug 181608: The only valid option in the File Types list is "Web Page, complete" but selecting this saves the page as Text, not HTML, does not create the _files directory or download any other files. It behaves like Save As Text. Save Page is totally broken. :(

Calling WideCharToMultiByte() with -1 for the Wstring will causes to convert the string only to the first null terminated string. Filter string is a pointer to a buffer containing pairs of null-terminated filter strings We need to find the correct filter string length before calling WideCharToMultiByte() and provide the length. http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winui/WinUI/WindowsUserInterface/UserInput/CommonDialogBoxLibrary/CommonDialogBoxReference/CommonDialogBoxStructures/OPENFILENAME.asp shanjian: can you review?

Comment on attachment 107420 [details] [diff] [review] Need to give a proper filter length to WideCharToMultiByte() Fix the nits and mark r=shanjian. >+ while ((aFileNameW->lpstrFilter[len] != L'\0') >+ || (aFileNameW->lpstrFilter[len+1] != L'\0')) >+ { >+ len += 1; >+ } while ((aFileNameW->lpstrFilter[len] != L'\0') || (aFileNameW->lpstrFilter[len+1] != L'\0')) { ++len; } >+ >+ len = WideCharToMultiByte(CP_ACP, 0, >+ aFileNameW->lpstrFilter, >+ len, >+ filterA, >+ MAX_FILTER_NAME, NULL, NULL); len = WideCharToMultiByte(CP_ACP, 0, aFileNameW->lpstrFilter, len, filterA, MAX_FILTER_NAME, NULL, NULL); >+ filterA[len] = '\0'; >+ filterA[len+1] = '\0'; > ofnA.lpstrFilter = filterA; > } > if (aFileNameW->lpstrCustomFilter) {

Comment on attachment 107642 [details] [diff] [review] Fixing the nits as per shanjian's comment.

Comment on attachment 107642 [details] [diff] [review] Fixing the nits ==== Why do we need to check |aFileNameW->lpstrFilter[len+1]!= L'\0'| too? If we ever hit that expression, that means that [len] was zero, so doesn't that mean we will be looking at a character beyond the string terminator? Also |L'\0'| is the same as |0| ... can't we just get away with checking |while(aFileNameW->lpstrFilter[len])|? + while ((aFileNameW->lpstrFilter[len] != L'\0') || + (aFileNameW->lpstrFilter[len+1] != L'\0')) + { + ++len; + }

Comment on attachment 107642 [details] [diff] [review] Fixing the nits Ok, duh, I read the URL provided by yokoyama above regarding the fact that the string is actually a collection of null terminated strings, plus an extra null at the end of the last string in the buffer, so disregard my questions above about the [len+1] check and the |strlen()|. So can we still remove the need for |L'\0'|? sr=kin@netscape.com

>So can we still remove the need for |L'\0'|? Thanks and I'll remove the |L'\0'|.

fix checked into the trunk.

We can choose types in "Save as type" drop-down box now. But it does not work at all. When type is changed, new type does not be effective. The previous saving type is still applyed. Reproducable on 2002-12-03-04-trunk (Win98SE Japanese).

Confirming comment #20 on win98 spanish with 2002120308, no matter what type it's selected, the pages is saved as html only. I suggest reopening this bug.

Confirming the behaviour in comment 20 and comment 21, build 2002120308 win95NO. Reopening.

changing the summary to reflect the real problem. I'll take a look.

nsFilePicker needs two elements back from the Open/Save dlgbox. lpstrFile and nFilterIndex. We missed the nsFilterIndex. shanjian: can you review?

Comment on attachment 108234 [details] [diff] [review] assign the new filter index kin: can you super review?

Comment on attachment 108234 [details] [diff] [review] assign the new filter index sr=kin@netscape.com

Comment on attachment 108234 [details] [diff] [review] assign the new filter index Impact: - Open/SaveAs dlgbox - Windows 9x platforms only

Comment on attachment 108234 [details] [diff] [review] assign the new filter index a=asa for checkin to 1.3a

fix checked in

Comment on attachment 108234 [details] [diff] [review] assign the new filter index > if (ofnA.lpstrFile) { > ConvertAtoW(ofnA.lpstrFile, MAX_PATH, aFileNameW->lpstrFile); > } Hey, I forget the bug number, but is this the code that stops you attaching multiple files to emails?

neil: not sure. I never knew such bug exists. If the code you point out is the cause, then the code only gets executed in Win9x. Is the bug only in Win9x?

neil: Never mind my previous question. You are correct. I have a patch ready for review; but I can't find the mail bug you are referring. Do you know the bug number? Seth: do you recall?

It's 180333. I'll attach a patch.

Since I don't have access to a Win 98 machine, could the reporter please verify this with the latest trunk ?

I can verify that this is now fixed on Windows 95/98.