- Wine-gitlab - list.winehq.org

[PATCH 0/4] MR9443: comctl32/tab: Add a helper to draw the border background.
by Zhiyi Zhang (＠zhiyi) 13 Nov '25

13 Nov '25

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9443

3 5

Re: [PATCH v4 0/1] MR9423: ntdll: Warn when environment block is too large, instead of removing it
by Martino Fontana (＠tinozzo123) 13 Nov '25

13 Nov '25

As it is right now, with _all_ envars removed, then `XDG_DATA_DIRS` is always removed as well, right? Then, what about first sorting for size, and then doing a stable sort on whether it's a special envar? (So that while special envars like `XDG_DATA_DIRS` become high priority, the big envars that aren't special remains at the lowest priority.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9423#note_121874

1 0

Re: [PATCH v3 0/1] MR9431: ole32: implement proper file flushing in FileLockBytes
by Esme Povirk (＠madewokherd) 13 Nov '25

13 Nov '25

Esme Povirk (@madewokherd) commented about dlls/ole32/filelockbytes.c: > > static HRESULT WINAPI FileLockBytesImpl_Flush(ILockBytes* iface) > { > + FileLockBytesImpl *This = impl_from_ILockBytes(iface); > + > + TRACE("(%p)\n", iface); > + > + /* verify a sane environment */ > + if (!This) return E_FAIL; > + > + if (!FlushFileBuffers(This->hfile)) > + return STG_E_WRITEFAULT; According to MSDN, this will fail if the file isn't opened with write permission (which, I think, shows that the tests aren't testing what you want). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9431#note_121872

1 0

Re: [PATCH v3 0/1] MR9431: ole32: implement proper file flushing in FileLockBytes
by Esme Povirk (＠madewokherd) 13 Nov '25

13 Nov '25

Esme Povirk (@madewokherd) commented about dlls/ole32/tests/storage32.c: > DeleteTestLockBytes(lockbytes); > } > > +static void test_lockbytes_flush_operation(void) > +{ > + static const WCHAR stmname[] = { 'T','E','S','T','S','T','R','E','A','M',0 }; I think we can just use `L"TESTSTREAM"` now. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9431#note_121870

1 0

Re: [PATCH v3 0/1] MR9431: ole32: implement proper file flushing in FileLockBytes
by Esme Povirk (＠madewokherd) 13 Nov '25

13 Nov '25

Esme Povirk (@madewokherd) commented about dlls/ole32/tests/storage32.c: > DeleteTestLockBytes(lockbytes); > } > > +static void test_lockbytes_flush_operation(void) > +{ > + static const WCHAR stmname[] = { 'T','E','S','T','S','T','R','E','A','M',0 }; > + TestLockBytes* lockbytes; > + HRESULT hr; > + IStorage* stg; > + IStream* stm; > + > + /* Test 1: Normal flush operation */ > + CreateTestLockBytes(&lockbytes); I don't think this tests what you want to test. `CreateTestLockBytes` uses the `TestLockBytes_Vtbl` implementation defined in the test itself. So calling the implementation of Flush defined in the test file doesn't really test anything about Windows. In fact, there doesn't seem to be a way to get the `ILockBytes` implementation from `filelockbytes.c` directly. I think that means that only the application-observable behavior matters. So, I think all we need the test to do is call `IStorage_Flush` on a storage file (and I think it needs to be a file, not an in-memory lockbytes) that was opened read-only. If that succeeds, then our internal ILockBytes should succeed in that case. If not, it'll depend on the behavior of `IStorage_Flush` in other situations. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9431#note_121871

1 0

Re: [PATCH v4 0/1] MR9423: ntdll: Warn when environment block is too large, instead of removing it
by Alexandre Julliard (＠julliard) 13 Nov '25

13 Nov '25

As has been pointed out, you can't decide which variables are important based on their size. Particularly since XDG_DATA_DIRS is one of the offenders, and it's used in Wine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9423#note_121869

1 0

Re: [PATCH v5 0/1] MR9418: winex11.drv: use proper timestamp for X11 clipboard selection
by Esme Povirk (＠madewokherd) 13 Nov '25

13 Nov '25

Esme Povirk (@madewokherd) commented about dlls/winex11.drv/x11drv_main.c: > "text/plain", > "text/rtf", > "text/richtext", > - "text/uri-list" > + "text/uri-list", > + "__wine_timestamp_prop" I think these are supposed to be sorted? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9418#note_121866

1 0

Re: [PATCH v5 0/1] MR9418: winex11.drv: use proper timestamp for X11 clipboard selection
by Esme Povirk (＠madewokherd) 13 Nov '25

13 Nov '25

Esme Povirk (@madewokherd) commented about dlls/winex11.drv/clipboard.c: > + XSelectInput( display, win, attrs.your_event_mask | PropertyChangeMask ); > + > + /* Generate a PropertyNotify event to get server time */ > + XChangeProperty( display, win, x11drv_atom(__wine_timestamp_prop), XA_INTEGER, 8, PropModeReplace, NULL, 0 ); > + XSync( display, False ); > + > + /* Wait for the PropertyNotify event */ > + for (i = 0; i < SELECTION_RETRIES; i++) > + { > + if(XCheckIfEvent( display, &event, timestamp_predicate, (XPointer)win )) > + { > + timestamp = event.xproperty.time; > + break; > + } > + selection_sleep(); > + } I'm not sure how we should handle the case where we don't see the event we're expecting. It's unclear to me from the documentation whether `XSync` should guarantee that we see the event (no other client is involved so.. maybe?). And I don't know whether we need a timeout or not, since we should get the event as long as the server is functioning properly and no one destroys the selection window in the small amount of time between creating it and calling this. It's also unclear to me whether we need to temporarily change our event selection, or whether we could just do it at window creation time. Since I'm not actually the maintainer in this area and am only reviewing out of interest anyway, I think it's best for me to just bring these up as a unresolved questions. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9418#note_121867

1 0

Re: [PATCH v4 0/1] MR9423: ntdll: Warn when environment block is too large, instead of removing it
by Martino Fontana (＠tinozzo123) 13 Nov '25

13 Nov '25

I'm fine on having a workaround, but not if it's too invasive, which IMO right now it is. So, I'm fine with only removing the biggest envars (which are much less likely to be important), like !6140 does It's just only around 40 lines added, and it has no performance impact if the envars block is small, so I don't really see why not that instead. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9423#note_121864

1 0

Re: [PATCH v4 0/1] MR9423: ntdll: Warn when environment block is too large, instead of removing it
by Alexandre Julliard (＠julliard) 13 Nov '25

13 Nov '25

Well, I don't think the warning makes any difference, so basically your solution is to do nothing and ignore the problem. That's certainly an option, but I was hoping we could do better... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9423#note_121863

1 0