Hi Ken,
2015-09-30 17:05 GMT+08:00 Ken Thomases <ken@codeweavers.com>:
>
> Since FlashWindowEx() is the more general function — that is, FlashWindow() could be fully implemented in terms of FlashWindowEx(), but not the other way around — shouldn't the driver entry point be FlashWindowEx()?  Otherwise, when FlashWindowEx() is eventually implemented more completely, the driver entry will have to be changed.
>
> For now, since user32 implements FlashWindow() but FlashWindowEx() is a stub (or semi-stub after your next patch), FlashWindow() would have to create a FLASHWINFO structure, fill it in, and pass that along.  I think that dwFlags would be either FLASHW_ALL or FLASHW_STOP, depending on bInvert.  uCount would be 1, I guess.
>
Thanks for your review!

Yes, function() will be implemented by functionEx() commonly in Wine.
But I didn't find a perfect way to implement FlashWindowEx() and then implement FlashWindow() with FlashWindowEx().
Because in X11, how many times will the window be flashed is depend on design of window manager/desktop environment.(For example, in XFCE, taskbar will always flashes when _NET_WM_STATE_DEMANDS_ATTENTION is sent until the user active the window again; In Ubuntu Unity, it'll only be flashed one time and then highlight taskbar until the user active the window again.) And I don't think it's necessary (and any possible?) to follow Window's design.
In this way, implementing uCount and other parameters may  be useless, because all we need to do is sending _NET_WM_STATE_DEMANDS_ATTENTION to WM/DE one time when FlashWindow()/FlashWindowEx() is called.

Similarly, Windows's FlashWindowEx() can decide which part it want to flash - only flash taskbar or only flash 
caption or flash both. But it'll depend on WM/DE's design in X11.

So I implemented FlashWindowEx() by calling FlashWindow() simply.

> Shouldn't pFlashWindow (or pFlashWindowEx) be added to the USER_DRIVER structure among the "windowing functions" (see the comments), in alphabetical order (after pDestroyWindow)?  Of course, that affects the changes to driver.c in terms of where things are added.  The order in winex11.drv.spec is less clear, but it seems to be mostly alphabetical after a certain point.  I'd put it after EnumClipboardFormats() in that list.
Agree.

>
> Why does FlashWindow() only call the driver entry point if the window is minimized?  Shouldn't the driver get the opportunity to act for a non-minimized window, too?
>
Same as above, different WM/DE may have different UI effect, that makes it hard to follow Windows's design.
In Windows, different UI effects will be showed when window is minimized and non-minimized.
And in my opinion, it is a little noisy if USER_Driver->pFlashWindow( hWnd, bInvert ) is called all the time.
So I'll prefer to only let the user notice new message when the window is minimized (it may hard to be noticed when
it's minimized).

> For the X11 implementation, shouldn't something potentially remove the _NET_WM_STATE_DEMANDS_ATTENTION state, presumably when bInvert is false (or when dwFlags doesn't include FLASHW_CAPTION or FLASHW_TRAY)?
You are right. It seems flashing will be stopped when bInvert is false. I'll improve it for this case.

Thanks again! :)

--
Regards,
Jactry Zeng