March 2014 - Wine-Devel - list.winehq.org

Re: [PATCH 3/5] d3d8/tests: Move some stateblock tests to device.c.

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://newtestbot.winehq.org/JobDetails.pl?Key=5656 Your paranoid android. === w8 (32 bit device) === device.c:2396: Test failed: Expected foreground window 0005025E, got 00010048. device.c:3038: Test failed: Expected wndproc 0x411180, got 0x411180.

March 13, 2014

Re: TestBot job 5654 results: [PATCH] msxml3: Respect stylesheet output mode when dumping resulting document

by Nikolay Sivov

On 3/13/2014 15:41, Marvin wrote: > > === w8 (32 bit domdoc) === > > msxml3: > domdoc.c:10385: Test failed: got 00000000 > domdoc.c:10399: Test failed: got 00000000 > domdoc.c:10451: Test failed: got 0 > domdoc.c:10476: Test failed: got 00000000 > domdoc.c:10550: Test failed: got 00000000 > domdoc.c:10565: Test failed: got 00000000 > domdoc.c:10619: Test failed: got 0 > domdoc.c:11268: Test failed: got 00000000 > domdoc.c:11294: Test failed: got 00000000 This is unrelated, I'll send a patch.

March 13, 2014

Re: [PATCH] msxml3: Respect stylesheet output mode when dumping resulting document

by Marvin

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://newtestbot.winehq.org/JobDetails.pl?Key=5654 Your paranoid android. === w8 (32 bit domdoc) === domdoc.c:10385: Test failed: got 00000000 domdoc.c:10399: Test failed: got 00000000 domdoc.c:10451: Test failed: got 0 domdoc.c:10476: Test failed: got 00000000 domdoc.c:10550: Test failed: got 00000000 domdoc.c:10565: Test failed: got 00000000 domdoc.c:10619: Test failed: got 0 domdoc.c:11268: Test failed: got 00000000 domdoc.c:11294: Test failed: got 00000000

March 13, 2014

Re: [PATCH 2/3] mshtml: Added IHTMLDocument7 stub implementation.

by Jacek Caban

On 03/12/14 19:05, Alexandre Julliard wrote: > Jacek Caban <jacek(a)codeweavers.com> writes: > >> Due to how tests are written, we need to switch to IWebBrowserPriv2IE9 >> in the same patch. >> >> --- >> dlls/ieframe/ieframe.h | 2 +- >> dlls/ieframe/navigate.c | 104 +---- >> dlls/mshtml/Makefile.in | 1 + >> dlls/mshtml/htmldoc.c | 4 + >> dlls/mshtml/htmldoc7.c | 939 +++++++++++++++++++++++++++++++++++++++++++ > Merging htmldoc3 and htmldoc5 was a good start, couldn't we do this with > the rest of the htmldoc implementations too? There are way too many > files in that dll already. Yes, we could, however: $ wc -l htmldoc*.c 2669 htmldoc.c 1294 htmldoc3.c 939 htmldoc7.c 4902 total Those are still not all interfaces implemented by HTMLDocument and most of functions are stubs. Assuming that, when implemented, all those functions will be trivial Gecko API wrappers (most of them will be, but some will be rather complex), that's about 20000 lines already. That's way too much for my taste. I may merge those files and revisit it at some point in the future. Jacek

March 13, 2014

Re: schedsvc: Add Task Scheduler service. Try 2.

by Dmitry Timoshkov

Alexandre Julliard <julliard(a)winehq.org> wrote: > >> > Would starting the service from taskschd.dll when an application calls > >> > ITaskService::Connect() be a viable compromise? Of course that would mean > >> > that the scheduled tasks will be added to the queue only when the service > >> > is running. > >> > >> If there are scheduled tasks then it's OK for the service to be running > >> always; but as long as there aren't, it should be started on-demand. > > > > So I would implement it like this: the service would be always started > > on Wine boot, it would then load the xml task definitions and if there is > > no anything would exit right away. Then the service would be started by > > taskschd.dll if ITaskService::Connect() can't connect to an existing > > service instance. Does this sound acceptable? > > No, having it start always defeats the whole point. Just mark it > auto-start once a task is added. In 'Try 3' I tried to address all the discussed things, is there anything else that prevents this patch from being accepted? -- Dmitry.

March 12, 2014

Re: Glorious winetestbot results, why this matters

by Juan Lang

On Wed, Mar 12, 2014 at 10:51 AM, Jacek Caban <jacek(a)codeweavers.com> wrote: > On 03/12/14 15:31, Jeremy White wrote: > > So here's today's data: > > http://www.winehq.org/~jwhite/31faed571d5a.html > > > > The main changes are the addition of some new failures in mshtml; > > failures that Marvin actually pointed out here: > > http://www.winehq.org/pipermail/wine-devel/2014-March/103419.html > > > > The glorious result is that if I get to publicly shame anyone, it's > > both Jacek and Alexandre. For someone with my JR, that's a rare > > opportunity <grin>. > > This should be fixed by my recent patches. The more worrying failure is > crypt32:chain. Those happened with no change in tests at all and they > don't happen on single test runs: > > https://testbot.winehq.org/JobDetails.pl?Key=5623 > https://testbot.winehq.org/JobDetails.pl?Key=5624 > > I sent a patch that should help diagnose the problem if failures will > continue to happen on full test runs. > Thanks, Jacek. That one's an oddball test failure. Random bad guess: certificate chain caching, including caching the chain verification results? Anyway, looking forward to getting more data on it. --Juan

March 12, 2014

Re: [RFC 5/11] Linux FF: Linux effect parameters: time parameters

by Elias Vanderstuyft

On Mon, Mar 10, 2014 at 5:19 PM, Andrew Eikum <aeikum(a)codeweavers.com> wrote: > On Sun, Mar 09, 2014 at 11:38:06AM +0100, Elias Vanderstuyft wrote: >> On Wed, Mar 5, 2014 at 4:03 PM, Andrew Eikum <aeikum(a)codeweavers.com> wrote: >> > On Tue, Mar 04, 2014 at 08:20:09PM +0100, Elias Vanderstuyft wrote: >> >> So change in dinput/effect_linuxinput.c:488 : >> >> This->effect.u.periodic.period = tsp->dwPeriod / 1000; >> >> to : >> >> if (0 < tsp->dwPeriod && tsp->dwPeriod <= 1000) >> >> This->effect.u.periodic.period = 1; >> >> else >> >> This->effect.u.periodic.period = tsp->dwPeriod / 1000; >> >> >> > >> > Seems good. While we're here, any reason not to use round(3) instead >> > of always rounding down in the >1000 case? >> >> That's a good question, the reason is that I want to avoid floating >> point arithmetic (round(3)). > > Okay. If using floating point is costly, then I agree the tiny (<1ms) > increase in accuracy isn't worth it. > >> > Is this how Windows behaves >> > for massive dwPeriod? I would expect E_INVALIDARG, since it's out of >> > the range [0,36000). >> >> I think you're mixing up dwPeriod with dwPhase. The range of dwPeriod >> is not defined in MSDN. >> > > Yes, you're right. Your updated patch seems fine. > >> Maybe we should also change all occurrences of 10000 in a force >> context to DI_FFNOMINALMAX, in the whole FF dinput implementation? >> > > I would support a patch like that, yes. OK. > > (I've been busy lately, but I'll get to the rest of your mail sequence > soon!) No problem, thanks for all feedback already! Elias

March 12, 2014

Re: [RFC 2/11] Linux FF: Linux effect status management

by Elias Vanderstuyft

On Mon, Mar 10, 2014 at 4:55 PM, Andrew Eikum <aeikum(a)codeweavers.com> wrote: > On Fri, Mar 07, 2014 at 10:23:18PM +0100, Elias Vanderstuyft wrote: >> On Wed, Mar 5, 2014 at 3:54 PM, Andrew Eikum <aeikum(a)codeweavers.com> wrote: >> > On Tue, Mar 04, 2014 at 08:19:39PM +0100, Elias Vanderstuyft wrote: >> >> => This has been discussed (see >> >> http://www.mail-archive.com/linux-input(a)vger.kernel.org/msg08513.html) >> >> and the following is true: >> >> My opinion appeared to be correct, but for to be sure, I was >> >> recommended to apply the change in Wine as well. >> >> >> > >> > Yes, we should also fix this in Wine to support "buggy" old kernels. >> > It would be worth adding a short comment to the code explaining which >> > kernel versions are affected by this bug. >> >> At the moment, I did not receive any reaction yet on my kernel patch >> (although it was suggested by one of the kernel devs): >> http://www.spinics.net/lists/linux-input/msg29920.html >> So I don't know yet in what kernel the bug will be resolved. >> If I my Wine patches make it first to release, what should I write >> then? "pending as of <current date>"? >> > > Oh, I didn't know it wasn't fixed yet. Maybe just briefly mention that > the parameter can be incorrectly modified by the kernel. The Git > commit date will point future developers to the correct timeframe. OK. So I have the following new code: LinuxInputEffectImpl *This = impl_from_IDirectInputEffect(iface); int effectId_old; TRACE("(this=%p)\n", This); effectId_old = This->effect.id; if (ioctl(*(This->fd), EVIOCSFF, &This->effect) == -1) { /* Restore effect id since this can be incorrectly modified by the kernel in case of upload failure. */ This->effect.id = effectId_old; > >> >> b) Be more precise in returning errors. >> >> >> > >> > Seems fine. If you can add tests (EINVAL seems easy to test, at >> > least), that would be even better. >> >> But that may be FF driver dependent, for example: >> - the (soon deprecated) ff-memless driver does not return an error >> when setting periodic effects' period to zero >> - the newer ff-memless-next driver does return an error when >> setting periodic effects' period to zero, and when setting other >> invalid parameters >> And I don't know whether Wine tests may rely on drivers? (normally, >> then you would also need to own a FF capable joystick) >> Unless you use a dummy-device, of course, but again, I don't think we >> want to include that in the Wine test tree, right? >> >> However on the other hand, I do understand that this should be tested. >> What do you suggest? >> > > To be clear, I meant testing dinput's behavior on Windows. If this > behavior changes on Windows based on which device you're using, then > you're correct, we can't really write sane tests for it. I only have Logitech FF joysticks, so I can't really tell. I remember on Windows, the Logitech drivers seem to fail silently. But as this might be driver dependent, I don't know what the best thing would be (, I would think that throwing an error is most logical: e.g. application developers should be noticed whether passing a specific parameter is invalid (with DIERR_INVALIDPARAM)) > >> >> c) The following in dinput/effect_linuxinput.c:336 : >> >> if (res != DI_OK) >> >> should be probably : >> >> if (FAILED(res)) >> >> for example if a device reports S_FALSE because it has already >> >> updated an identical effect. >> >> >> >> The same for line 549: >> >> if (retval != DI_OK) >> >> should be then : >> >> if (FAILED(retval)) >> >> >> > >> > Sure, seems fine. As an additional fix, you could demonstrate with >> > tests that Download and Start can return S_FALSE under those >> > circumstances on Windows, and fix the Wine behavior to match. >> >> I'm new to Wine's test-framework, suppose I managed to create such >> test and successfully run it on Wine, how can I compile that test for >> Windows? (I don't have MS VS, and I prefer to not use it) > > There's instructions for cross compiling Wine tests here: > http://wiki.winehq.org/CompilingDLLsUsingMingw > In short, after installing the correct cross-compiler, just run "make > crosstest" in the DLL's tests directory. Thanks, very useful! > >> (Maybe I should also mention that I don't have enough time for the >> moment to create tests.) >> > > Many of your patches don't need tests, as they're obviously correct or > are making corrections to Linux-specific behavior. There are some > patches where you need tests to determine correct Windows behavior. Alright. > > You could create a bug in Bugzilla and attach your patches there, > mentioning what tests need to be written. Then the patches won't be > lost on the mailing list, and perhaps another developer could take the > time to write the tests confirming your changes. Thanks for the advice. Elias

March 12, 2014

Re: [RFC 1/11] Linux FF: Joystick control panel program

by Elias Vanderstuyft

On Mon, Mar 10, 2014 at 4:47 PM, Andrew Eikum <aeikum(a)codeweavers.com> wrote: > > On Fri, Mar 07, 2014 at 10:01:30PM +0100, Elias Vanderstuyft wrote: > > On Wed, Mar 5, 2014 at 3:49 PM, Andrew Eikum <aeikum(a)codeweavers.com> wrote: > > > On Tue, Mar 04, 2014 at 08:19:16PM +0100, Elias Vanderstuyft wrote: > > >> a) When unloading conditional effects in joy.cpl, the program crashes: > > >> When unloading the conditional effect array entries, > > >> Wine tries to access an invalid (high) address because > > >> dieffect.lpvTypeSpecificParams was not set at the time the > > >> effect was created. > > >> So declare a valid 'standard' (maxed out) conditional effect. > > >> For other unknown effects: > > >> - The same can happen when CustomForce effects are available, > > >> but because Wine's FF dinput->linux translation does > > >> not support this yet, > > >> we simply print a FIXME to indicate a possible cause > > >> if a crash would happen. > > >> - The same for other (unsupported) effects (this is > > >> probably not safe?), > > >> but then we print a WARN. > > > > > > For unhandled types, it's probably best to print a FIXME and return > > > DIENUM_CONTINUE, rather than continue to rely on uninitialized data. > > > > Alright. > > Should I also write the extra if-test for the CustomForce type, or do > > I have to cover it with one if-test as an unhandled type? > > > > I'd just treat it as an unhandled type. OK. > > Before I knew the joy.cpl app was to be blamed, and was searching in > > the IDirectInputEffect implementions, I saw something that is not > > conform MSDN: > > JoystickAImpl_EnumEffects and JoystickWImpl_EnumEffects should check > > on DIENUM_CONTINUE to continue the enumeration, or DIENUM_STOP to stop > > it: > > http://msdn.microsoft.com/en-us/library/windows/desktop/microsoft.directx_s… > > For example, in JoystickWImpl_EnumEffects(), each occurrence of > > "(*lpCallback)(&dei, pvRef);" should check whether the return value is > > DIENUM_CONTINUE or DIENUM_STOP. When the latter is the case, should we > > then first add a "goto" statement to jump to the last "if (xfd == -1) > > IDirectInputDevice8_Unacquire(iface);" statement, or directly return? > > And what should it return in that case, "DIERR_NOTINITIALIZED" (does > > stopping enumeration mean failure? that's not clear in MSDN), or > > "DI_OK"? > > Probably the goto. I'm not sure about the return value, you would need > tests to be certain. Probably DI_OK is correct. OK, I'll queue all patches that need testing (and open a bug report if I really don't have enough time to create the tests by myself.) For the other patches, I'll post them as soon as possible to wine-patches. > > > And even worse, what if the return value of "(*lpCallback)(&dei, > > pvRef);" is not equal to DIENUM_CONTINUE or DIENUM_STOP? Should > > JoystickWImpl_EnumEffects() interpret it as the former or as the > > latter? (also not mentioned in MSDN) > > > > Again, you'd need tests to confirm. OK. > > > dieffect.cAxes = 2; > > dieffect.rgdwAxes = axes; > > dieffect.rglDirection = direction; > > > > DIRAMPFORCE rforce; > > DICONSTANTFORCE cforce; > > DIPERIODIC pforce; > > DICONDITION cxyforce[2]; > > > > if (IsEqualGUID(&pdei->guid, &GUID_RampForce)) > > > > Yes, but the declarations have to be at the very top of the function, > alongside all of the other declarations. Oh, thanks for noticing, I should have known that ;) Elias

March 12, 2014

Re: [5/6] user32: Set capture on the owner of popup menus.

by Alexandre Julliard

Vincent Povirk <madewokherd(a)gmail.com> writes: > @@ -2057,6 +2057,19 @@ BOOL CDECL X11DRV_ScrollDC( HDC hdc, INT dx, INT dy, HRGN update ) > } > > > +BOOL CALLBACK find_popup_menu_window(HWND hwnd, LPARAM lparam) > +{ > + HWND *result = (HWND*)lparam; > + > + if (GetClassLongW(hwnd, GCW_ATOM) == MAKEINTATOM(32768)) > + { > + *result = hwnd; > + return FALSE; > + } That's ugly and unreliable. -- Alexandre Julliard julliard(a)winehq.org

March 12, 2014