[Bug 19495] New: DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

Summary: DialogBox() returns -1 for Invalid Window Handle,
                    Should Be 0
           Product: Wine
           Version: 1.1.26
          Platform: PC
        OS/Version: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: user32
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]

DialogBox( hInstance, MAKEINTRESOURCE(MyDlg), NULL, MyDlgProc )
returns -1 (AFTER the dialog has completed correctly!)
GetLastError() returns 1400 (Invalid Window Handle).
NULL is the window handle, meaning the desktop is the owner.

According to the MS docs here
http://msdn.microsoft.com/en-us/library/ms645452(VS.85).aspx
"If the function fails because the hWndParent parameter is invalid, the return
value is zero."

I can work around this, but it might be causing problems in commercial
software.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #1 from Juan Lang   2009-07-28 18:15:39 ---
Please write a test case demonstrating the problem.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #2 from Dmitry Timoshkov   2009-07-28 22:41:01 ---
There are some existing tests for DialogBoxParam in dlls/user32/tests/dialog.c,
add your test there.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #3 from [e-mail]  2009-07-29 17:41:35 ---
Created an attachment (id=22692)
 --> (http://bugs.winehq.org/attachment.cgi?id=22692)
Test Case C Source for DialogBox Returns -1 bug

winedlg.rc has been added to the C source as a comment at the bottom.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #4 from [e-mail]  2009-07-29 17:44:09 ---
Created an attachment (id=22693)
 --> (http://bugs.winehq.org/attachment.cgi?id=22693)
Resource Source for DialogBox Returns -1 bug

The .rc that was included as a comment in the C code.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #5 from [e-mail]  2009-07-29 17:47:00 ---
I have no idea where "dlls/user32/tests/dialog.c" is located, so wrote a test
app.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

Vitaliy Margolen  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #22692|text/x-csrc                 |text/plain
          mime type|                            |

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #6 from Vitaliy Margolen   2009-07-29 21:29:07 ---
(In reply to comment #5)
> I have no idea where "dlls/user32/tests/dialog.c" is located.

http://source.winehq.org/git/wine.git/?a=blob;f=dlls/user32/tests/dialog.c

Unfortunately your app is not an automated test, it can not be included into
Wine's test framework.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #22692|0                           |1
        is obsolete|                            |

--- Comment #7 from [e-mail]  2009-08-01 17:38:30 ---
Created an attachment (id=22770)
 --> (http://bugs.winehq.org/attachment.cgi?id=22770)
Framework Test Case C Source for DialogBox Returns -1 bug

It is unfortunate you cannot fix bugs without using the test framework.

I hope this "test framework" code works properly as I don't seem to have the
whole thing to confirm.

I reused the dialog resource from another test case.
If that is bad, then just use the one I already posted.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #8 from Dmitry Timoshkov   2009-08-02 03:27:24 ---
(In reply to comment #7)
> It is unfortunate you cannot fix bugs without using the test framework.

It's unfortunate that your "test framework" doesn't test anything, and
contains so much errors. Have you tried it at least compile, I don't even
mention to run?

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
          Component|user32                      |-unknown
         Resolution|                            |INVALID

--- Comment #9 from Dmitry Timoshkov   2009-08-03 22:36:12 ---
I'm going to mark this bug as invalid unless there is a clear test case
which shows the bug. NULL is not an invalid window handle as the reporter
of the bug implies, and the "test case" is broken.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #10 from Dmitry Timoshkov   2009-08-03 22:36:39 ---
Closing invalid.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|CLOSED                      |UNCONFIRMED
         Resolution|INVALID                     |

--- Comment #11 from [e-mail]  2009-08-04 14:24:53 ---
(In reply to comment #8)
> It's unfortunate that your "test framework" doesn't test anything, and
> contains so much errors. Have you tried it at least compile, I don't even
> mention to run?

I don't have your test harness.
My test code was compiled with BCB 5.5 and then modified to fit into your
framework as best as I could understand it.

I tested everything in the DlgProc.
I tested everything in the TestProc except for
1) The dialog resource which I just reused one from another test case, as all I
needed was a valid dialog.
2) I guessed at the functionality of ok().

It does test the bug.
How does it "not test anything" ?

The bug is in the return value of DialogBox when an hWindow of 0 is passed.
The DlgProc closes the dialog as soon as it opens. This automates the user
pressing a close button. We aren't interested in what the dialog does.
The interesting item is the return value of -1, when it should be 0.
> NULL is not an invalid window handle as the reporter
> of the bug implies, and the "test case" is broken.

According to the docs, NULL is invalid, and a special case has been made to
return a 0 instead of -1 in that case.
Are you saying that DialogBox( inst, resource, NULL, proc ) is now returning 0?
The version of Wine I have here clearly returns -1 and errorcode of 1400.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

André H.  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #12 from André H.   2009-08-04 14:53:56 ---
maybe someone should have point you to:
http://wiki.winehq.org/ConformanceTests

and how to get the source of wine where "dlls/user32/tests/dialog.c" is
located:
http://wiki.winehq.org/GitWine

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID

--- Comment #13 from Dmitry Timoshkov   2009-08-04 21:57:06 ---
Comment #6 has the pointer to the test source.

hwnd 0 is a notion of a desktop window (HWND_DESKTOP), it's a valid pseudo
window handle.

Regarding to your test case:

static int test_DialogBoxReturnProc(HWND hWnd, unsigned message, unsigned
wParam, LONG lParam)

dialog callback has wrong calling convention and wrong parameter types.
If you would try to compile it you would notice that.

db = DialogBoxA(g_hinst, "RADIO_TEST_DIALOG", NULL, test_DialogBoxReturnProc );
le = GetLastError();
if( -1 == db )
{
    ok (le == 1400, "DialogBox Failed to return 0.\n");
    ok (le != 1400, "test_DialogBoxReturn failed for unknow reason.\n");
}

First of all DialogBoxA() will never fail, therefore the code under if ()
statement will never ececute. Second which of ok() calls above is supposed
to test what you want?

Again, reopen once you have an evidence of the bug, and an appropriate test
case.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #14 from Dmitry Timoshkov   2009-08-04 21:57:46 ---
Closing.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|CLOSED                      |UNCONFIRMED
         Resolution|INVALID                     |

--- Comment #15 from [e-mail]  2009-08-04 22:27:34 ---
(In reply to comment #12)
> maybe someone should have point you to:
> http://wiki.winehq.org/ConformanceTests

"How to run conformance tests."
Seems to tell me (not very well) how to download and run a test exe.
It says nothing about adding a new test.

OTOH, your link takes me one step deeper to
http://wiki.winehq.org/MakeTestFailures
Where it says I shouldn't even try to do what you-all are asking:
"(Note: building Wine on 64 bit systems can be difficult. If you have a 64 bit
system, and you haven't already successfully built wine, please don't try just
to help out with this, it's too much trouble.)"
> and how to get the source of wine where "dlls/user32/tests/dialog.c" is
> located:
> http://wiki.winehq.org/GitWine

"Using Git to maintain your patches against Wine"
I suppose that means I use git to get the test code?
I'm not much interested in recompiling wine or patching it.
Or in learning how to use git. I'm a one-man shop. User tells me about a bug, I
go in and fix it. I don't ask the user to learn programming and git and then
write a conformance test for me.

I followed all 6 steps on the Bugzilla page, and then filed a detailed bug
report including steps to reproduce the bug.
I've provided two codes, the first one a complete executable, demonstrating the
bug. The second, the nearest I could make out, a conformance test to be added
into the dialog test by the keepers of the test.
I guess no one has bothered to run the first full executable or this wouldn't
still be "unconfirmed".

I've done my job, twice over.
I've coded around your bug.
Commercial software probably doesn't code around it, so I figure it might be
causing problems that you haven't figured out yet.
I can't do your job for you. But I can point out failings as they come up.

I am truly sorry the Wine Bug Whackers aren't interested in knowing about
actual bugs.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #16 from [e-mail]  2009-08-04 22:53:43 ---
(In reply to comment #13)
> Comment #6 has the pointer to the test source.
>
> hwnd 0 is a notion of a desktop window (HWND_DESKTOP), it's a valid pseudo
> window handle.

Correct.
Did you try the code, or are you just desk-checking it?
Your expectations are my expectations, but reality is different.

I shall re-post the Microsoft entry on this subject:
http://msdn.microsoft.com/en-us/library/ms645452(VS.85).aspx
If the function fails because the hWndParent parameter is invalid, the return
value is zero. The function returns zero in this case for compatibility with
previous versions of Microsoft Windows. If the function fails for any other
reason, the return value is -1. To get extended error information, call 
GetLastError.

Fact: DialogBox returns -1
Fact: GetLastError returns 1400
This is not as-documented.
My conclusion is two-fold:
1) 0 is treated by wine as invalid (assumption).
2) wine is bugged on the return code (seemingly fact).
> Regarding to your test case:
>
> static int test_DialogBoxReturnProc(HWND hWnd, unsigned message, unsigned
> wParam, LONG lParam)
>
> dialog callback has wrong calling convention and wrong parameter types.
> If you would try to compile it you would notice that.

My compiler is less strict than yours.
It allows the substitution of post-macro types.
> db = DialogBoxA(g_hinst, "RADIO_TEST_DIALOG", NULL, test_DialogBoxReturnProc );
> le = GetLastError();
> if( -1 == db )
> {
>     ok (le == 1400, "DialogBox Failed to return 0.\n");
>     ok (le != 1400, "test_DialogBoxReturn failed for unknow reason.\n");
> }
>
> First of all DialogBoxA() will never fail,

You ASUSUME it won't fail.
It DOES! That's the point.
It fails (returns -1) when it shouldn't (should return 0);
> therefore the code under if ()
> statement will never ececute.

But it does. If you would ever actually try it!
> Second which of ok() calls above is supposed
> to test what you want?

The first is the proper test.
1400 is illegal window handle, which you will see if you ever run it.

The second is backup in case something else, like resource not available crops
up.
> Again, reopen once you have an evidence of the bug, and an appropriate test
> case.

I have all the evidence I need.
But just to be complete, I will correct the proc, and remove the second ok to
make you happy.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #17 from Dmitry Timoshkov   2009-08-04 22:59:17 ---
> > First of all DialogBoxA() will never fail,
>
> You ASUSUME it won't fail.
> It DOES! That's the point.
> It fails (returns -1) when it shouldn't (should return 0);

It doesn't fail here. Did you link the resources in?

Your bug report talks about an invalid window handle, but your test
doesn't even try to pass an invalid handle. What are you trying to test?

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #18 from [e-mail]  2009-08-04 23:01:07 ---
Created an attachment (id=22819)
 --> (http://bugs.winehq.org/attachment.cgi?id=22819)
 Framework Test Case C Source for DialogBox Returns -1 bug (2nd try)

Fixed the bugs you complained about

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #19 from [e-mail]  2009-08-04 23:05:53 ---
(In reply to comment #17)
> > > First of all DialogBoxA() will never fail,
> >
> > You ASUSUME it won't fail.
> > It DOES! That's the point.
> > It fails (returns -1) when it shouldn't (should return 0);
>
> It doesn't fail here. Did you link the resources in?

A lost resource doesn't return 1400.
> Your bug report talks about an invalid window handle, but your test
> doesn't even try to pass an invalid handle. What are you trying to test?

You and I both think 0 is a valid handle.
But WINE complains.
It returns -1 and 1400.

Would you like my original exe?

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #20 from [e-mail]  2009-08-04 23:08:13 ---
Created an attachment (id=22821)
 --> (http://bugs.winehq.org/attachment.cgi?id=22821)
Executable of my first 2 attachments

Compiled from my first 2 attachments (not your test harness).

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #21 from Dmitry Timoshkov   2009-08-04 23:21:45 ---
> You and I both think 0 is a valid handle.
> But WINE complains.
> It returns -1 and 1400.

How do you see that Wine complains? Everything runs fine here.

And once again:

Your bug report talks about an invalid window handle, but your test
doesn't even try to pass an invalid handle. What are you trying to test?

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID

--- Comment #22 from Dmitry Timoshkov   2009-08-05 00:53:36 ---
Once you will look at the Wine source you see that DialogBoxParam()
implementation returns -1 only in one case - when its can't find
the specified resource.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #23 from Dmitry Timoshkov   2009-08-05 00:55:02 ---
Closing again.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #24 from André H.   2009-08-05 11:43:21 ---
@Robert:
MSDN is often wrong. Its not always what Windows really does. Thats a point you
should see.
building Wine on 64 bit systems is not difficult. that was just an old phrase,
i fixed that.
I never wanted to attack you, i just wanted to say would you be so good as to
add a test into the winecode.

@Dmitry:
please stop closing the bug all the time, it is disrespectful if we still
discuss it

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #25 from Dmitry Timoshkov   2009-08-05 23:05:18 ---
(In reply to comment #24)
> @Robert:
> MSDN is often wrong. Its not always what Windows really does. Thats a point you
> should see.

MSDN describes that behaviour correctly, and Wine implements it that way.
> @Dmitry:
> please stop closing the bug all the time, it is disrespectful if we still
> discuss it

Personally I don't discuss this bug, to me there is no any bug, the reporter
is confused, and can't even show what doesn't work for him. And I explained
why current Wine source can't cause that behaviour.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #26 from Andrew Nguyen   2009-08-06 06:30:51 ---
Created an attachment (id=22858)
 --> (http://bugs.winehq.org/attachment.cgi?id=22858)
user32/tests: Test the behavior of DialogBoxParamA with dialog procedure window
destruction.

There is an actual difference in behavior between Wine and Windows, but it's
not due to the parameters passed into DialogBoxParamA. Rather, the actual issue
is the different handling of dialogs that are destroyed by their dialog
procedure, rather than by DialogBoxParamA. While MSDN states that a dialog
procedure should call EndDialog to destroy a dialog, Robert's dialog procedure
calls DestroyWindow on the dialog window handle instead:

switch (message) {
case WM_INITDIALOG:
    PostMessage( hWnd, WM_CLOSE, 0, 0 );
    return 1;
case WM_CLOSE:
    DestroyWindow(hWnd);
    return 0;
case WM_DESTROY:
    PostQuitMessage(3);
    return 0;
default:
    break;
};

Wine's DialogBoxParamA doesn't handle this in the same manner as Windows. Line
812 of DIALOG_DoDialogBox() does:

if (!IsWindow( hwnd )) return -1;

Possibly the return value should be revised to 0. I've attached a test case
demonstrating the issue.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #27 from Dmitry Timoshkov   2009-08-06 10:01:13 ---
Please send the test to wine-patches, I'll have a look at it once it's
committed.

[Bug 19495] DialogBox() returns -1 for Invalid Window Handle, Should Be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

--- Comment #28 from Andrew Nguyen   2009-08-07 04:07:01 ---
I submitted the tests:

http://www.winehq.org/pipermail/wine-patches/2009-August/076761.html

[Bug 19495] DialogBox() returns -1 when dialog destroys itself on WM_CLOSE, should be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|CLOSED                      |UNCONFIRMED
         Resolution|INVALID                     |
            Summary|DialogBox() returns -1 for  |DialogBox() returns -1 when
                   |Invalid Window Handle,      |dialog destroys itself on
                   |Should Be 0                 |WM_CLOSE, should be 0

--- Comment #29 from Dmitry Timoshkov   2009-08-07 09:18:26 ---
Thanks Andrew for the test. Reopening.

[Bug 19495] DialogBox() returns -1 when dialog destroys itself on WM_CLOSE, should be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

Dmitry Timoshkov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED

--- Comment #30 from Dmitry Timoshkov   2009-08-12 22:27:31 ---
Should be fixed by
http://source.winehq.org/git/wine.git/?a=commit;h=96e44ddb78950b55ab45d91fb8...

[Bug 19495] DialogBox() returns -1 when dialog destroys itself on WM_CLOSE, should be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

Alexandre Julliard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #31 from Alexandre Julliard   2009-08-21 12:54:36 ---
Closing bugs fixed in 1.1.28.

[Bug 19495] DialogBox() returns -1 when dialog destroys itself on WM_CLOSE, should be 0

http://bugs.winehq.org/show_bug.cgi?id=19495

Anastasius Focht  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
      Fixed by SHA1|                            |96e44ddb78950b55ab45d91fb8a
                   |                            |3231c58879294
                 CC|                            |[e-mail]
          Component|-unknown                    |user32