Re: Wine Gecko 2.24-beta1
On 08/18/13 04:14, Zhenbo Li wrote: I've tested some websites, and found that these pages have problems: http://mail.163.com (it will make gecko crash) http://huaban.com (nothing can be shown) http://douban.com (no pictures) http://html5test.com/ (can't load page, and I'm sure that it's not a regression) I've tested them with Firefox + flashblock, so I think they are not caused by flash. I'm glad to do more tests to contribute Gecko. Thanks for testing. I confirmed all those bugs while preparing the release, but they seem to be caused by MSHTML, not Gecko itself, so the release won't change behaviour here. Cheers, Jacek
Re: wininet: Don't assume that end of chunk means end of stream. (try 2)
Hi Hans,
Sorry for the delay, I wanted to test it a bit more before answering.
On 09/23/13 10:50, Hans Leidekker wrote:
On Thu, 2013-09-19 at 17:38 +0200, Jacek Caban wrote:
I was hoping for a test like the attached one, which shows that there is
still more work to do. But it's a step in the right direction, so I'm
fine with your patch.
The issue here is that HTTP_ReceiveRequestData calls refill_read_buffer, which
calls generic read_http_stream with a read size of 8192 (the read buffer
is still empty). In non-chunked mode this should read the minimum of that size
and the content length. In chunked mode we don't know the content length,
and we should read the minimum of chunk size and read buffer size, as shown by
your test.
Maybe we should add a refill_read_buffer method to the backends and call that
instead of read_http_stream?
We should be able to do that with read semantics that is intended for
READMODE_ASYNC: read anything, not more than buf size and, once any data
is read, don't block. The attached extended tests show the problem with
chunked reads in this case. We'd need to maintain full state of the
stream to be able to do non-blocking chunk headers/tails reads.
Jacek
diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c
index 12eab57..889707c 100644
--- a/dlls/wininet/tests/http.c
+++ b/dlls/wininet/tests/http.c
@@ -103,7 +103,7 @@ static int expect[MAX_INTERNET_STATUS],
optional[MAX_INTERNET_STATUS],
wine_allow[MAX_INTERNET_STATUS], notified[MAX_INTERNET_STATUS];
static const char *status_string[MAX_INTERNET_STATUS];
-static HANDLE hCompleteEvent, conn_close_event;
+static HANDLE hCompleteEvent, conn_close_event, server_event;
static DWORD req_error;
#define TESTF_REDIRECT 0x01
@@ -2114,6 +2114,36 @@ static DWORD CALLBACK server_thread(LPVOID param)
}
if (strstr(buffer, GET /test_premature_disconnect))
trace(closing connection\n);
+if (strstr(buffer, GET /test_chunked)) {
+static const char chunked_header[] = HTTP/1.1 200
OK\r\nTransfer-Encoding: chunked\r\n\r\n;
+static const char chunk1[] =
20\r\n\r\n;
+static const char chunk2[] = 18\r\n\r\n;
+static const char chunk3[] =
28\r\n\r\n;
+static const char last_chunk[] = 0\r\n\r\n;
+
+send(c, chunked_header, sizeof(chunked_header)-1, 0);
+send(c, chunk1, sizeof(chunk1)-1, 0);
+WaitForSingleObject(server_event, INFINITE);
+
+send(c, chunk2, sizeof(chunk2)-1, 0);
+WaitForSingleObject(server_event, INFINITE);
+
+send(c, chunk3, sizeof(chunk3)-1, 0);
+WaitForSingleObject(server_event, INFINITE);
+
+send(c, chunk1, sizeof(chunk1)-1, 0);
+send(c, chunk2, sizeof(chunk2)-1, 0);
+WaitForSingleObject(server_event, INFINITE);
+
+send(c, chunk1, sizeof(chunk1)-1, 0);
+send(c, chunk2, 3, 0);
+WaitForSingleObject(server_event, INFINITE);
+
+send(c, chunk2+3, sizeof(chunk2)-4, 0);
+WaitForSingleObject(server_event, INFINITE);
+
+send(c, last_chunk, sizeof(last_chunk)-1, 0);
+}
shutdown(c, 2);
closesocket(c);
@@ -2917,6 +2947,181 @@ static void test_no_content(int port)
CloseHandle(hCompleteEvent);
}
+static void test_chunked_stream(int port)
+{
+HINTERNET session, connection, req;
+DWORD res, avail, size;
+BYTE buf[256];
+
+trace(Testing chunked stream...\n);
+
+hCompleteEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
+server_event = CreateEvent(NULL, FALSE, FALSE, NULL);
+
+session = InternetOpenA(, INTERNET_OPEN_TYPE_PRECONFIG, NULL, NULL,
INTERNET_FLAG_ASYNC);
+ok(session != NULL,InternetOpen failed with error %u\n, GetLastError());
+
+pInternetSetStatusCallbackA(session, callback);
+
+SET_EXPECT(INTERNET_STATUS_HANDLE_CREATED);
+connection = InternetConnectA(session, localhost, port,
+NULL, NULL, INTERNET_SERVICE_HTTP, 0x0, 0xdeadbeef);
+ok(connection != NULL,InternetConnect failed with error %u\n,
GetLastError());
+CHECK_NOTIFIED(INTERNET_STATUS_HANDLE_CREATED);
+
+SET_EXPECT(INTERNET_STATUS_HANDLE_CREATED);
+req = HttpOpenRequestA(connection, GET, /test_chunked, NULL, NULL,
NULL,
+INTERNET_FLAG_KEEP_CONNECTION | INTERNET_FLAG_RESYNCHRONIZE,
0xdeadbead);
+ok(req != NULL, HttpOpenRequest failed: %u\n, GetLastError());
+CHECK_NOTIFIED(INTERNET_STATUS_HANDLE_CREATED);
+
+trace(sending request...\n);
+
+SET_OPTIONAL(INTERNET_STATUS_COOKIE_SENT);
+SET_EXPECT(INTERNET_STATUS_CONNECTING_TO_SERVER);
+SET_EXPECT(INTERNET_STATUS_CONNECTED_TO_SERVER);
+SET_EXPECT(INTERNET_STATUS_SENDING_REQUEST);
+SET_EXPECT(INTERNET_STATUS_REQUEST_SENT);
+SET_EXPECT
Re: add ualapi directory (try 9
Hi Akira, On 09/23/13 10:00, Akira Nakagawa wrote: From 2ecc71fdb3406d3cf79ebcbd7f6d0c711b68d699 Mon Sep 17 00:00:00 2001 Message-Id: 2ecc71fdb3406d3cf79ebcbd7f6d0c711b68d699.1379923234.git.matyapir...@gmail.com From: Akira Nakagawa matyapir...@gmail.com Date: Mon, 23 Sep 2013 17:00:04 +0900 Subject: [PATCH] add ual directory (try9) --- configure | 2 + configure.ac| 2 + dlls/ualapi/Makefile.in | 6 +++ dlls/ualapi/main.c | 86 + dlls/ualapi/tests/Makefile.in | 8 dlls/ualapi/tests/ualapi_test.c | 38 ++ dlls/ualapi/ualapi.spec | 4 ++ include/Makefile.in | 1 + include/ual.h | 48 +++ 9 files changed, 195 insertions(+) Please don't include tests in this patch. Tests won't be committed until they are useful, so send them once you have something useful in them and as separated patch. For now, just strip them from the patch. Also, you may consider sending public header (include/ual.h) as a separated patch. And you don't need to include changes to generated files (configure, configure.ac and include/Makefile.in). Cheers, Jacek
Re: wininet: Don't assume that end of chunk means end of stream. (try 2)
Hi Hans,
On 09/18/13 13:40, Hans Leidekker wrote:
diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c
index bf57223..2526681 100644
--- a/dlls/wininet/tests/http.c
+++ b/dlls/wininet/tests/http.c
@@ -4777,8 +4777,37 @@ static const struct notification
async_send_request_ex_resolve_failure_test[] =
{ internet_close_handle, INTERNET_STATUS_HANDLE_CLOSING, 0, }
};
+static const struct notification async_send_request_ex_chunked_test[] =
+{
+{ internet_connect, INTERNET_STATUS_HANDLE_CREATED },
+{ http_open_request, INTERNET_STATUS_HANDLE_CREATED },
+{ http_send_request_ex, INTERNET_STATUS_DETECTING_PROXY, 1, 0, 1 },
+{ http_send_request_ex, INTERNET_STATUS_COOKIE_SENT, 1, 0, 1 },
+{ http_send_request_ex, INTERNET_STATUS_RESOLVING_NAME, 1, 0, 1 },
+{ http_send_request_ex, INTERNET_STATUS_NAME_RESOLVED, 1, 0, 1 },
+{ http_send_request_ex, INTERNET_STATUS_CONNECTING_TO_SERVER, 1 },
+{ http_send_request_ex, INTERNET_STATUS_CONNECTED_TO_SERVER, 1 },
+{ http_send_request_ex, INTERNET_STATUS_SENDING_REQUEST, 1 },
+{ http_send_request_ex, INTERNET_STATUS_REQUEST_SENT, 1 },
+{ http_send_request_ex, INTERNET_STATUS_REQUEST_COMPLETE, 1 },
+{ http_end_request, INTERNET_STATUS_RECEIVING_RESPONSE, 1 },
+{ http_end_request, INTERNET_STATUS_RESPONSE_RECEIVED, 1 },
+{ http_end_request, INTERNET_STATUS_REQUEST_COMPLETE, 1 },
+{ internet_close_handle, INTERNET_STATUS_CLOSING_CONNECTION },
+{ internet_close_handle, INTERNET_STATUS_CONNECTION_CLOSED },
+{ internet_close_handle, INTERNET_STATUS_HANDLE_CLOSING },
+{ internet_close_handle, INTERNET_STATUS_HANDLE_CLOSING }
+};
+
static const struct notification_data notification_data[] = {
{
+async_send_request_ex_chunked_test,
+
sizeof(async_send_request_ex_chunked_test)/sizeof(async_send_request_ex_chunked_test[0]),
+GET,
+test.winehq.org,
+tests/data.php
+},
+{
async_send_request_ex_test,
sizeof(async_send_request_ex_test)/sizeof(async_send_request_ex_test[0]),
POST,
@@ -5100,6 +5129,7 @@ START_TEST(http)
test_async_HttpSendRequestEx(notification_data[0]);
test_async_HttpSendRequestEx(notification_data[1]);
test_async_HttpSendRequestEx(notification_data[2]);
+test_async_HttpSendRequestEx(notification_data[3]);
InternetOpenRequest_test();
test_http_cache();
InternetOpenUrlA_test();
I was hoping for a test like the attached one, which shows that there is
still more work to do. But it's a step in the right direction, so I'm
fine with your patch.
Thanks,
Jacek
commit 39b90723d968f65d1ad11a0c881fb9e7433c6fb3
Author: Jacek Caban ja...@codeweavers.com
Date: Thu Sep 19 17:18:29 2013 +0200
wininet: Added more chunked stream tests.
diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c
index bf57223..fb67604 100644
--- a/dlls/wininet/tests/http.c
+++ b/dlls/wininet/tests/http.c
@@ -103,7 +103,7 @@ static int expect[MAX_INTERNET_STATUS],
optional[MAX_INTERNET_STATUS],
wine_allow[MAX_INTERNET_STATUS], notified[MAX_INTERNET_STATUS];
static const char *status_string[MAX_INTERNET_STATUS];
-static HANDLE hCompleteEvent, conn_close_event;
+static HANDLE hCompleteEvent, conn_close_event, server_event;
static DWORD req_error;
#define TESTF_REDIRECT 0x01
@@ -2113,6 +2113,25 @@ static DWORD CALLBACK server_thread(LPVOID param)
}
if (strstr(buffer, GET /test_premature_disconnect))
trace(closing connection\n);
+if (strstr(buffer, GET /test_chunked)) {
+static const char chunked_header[] = HTTP/1.1 200
OK\r\nTransfer-Encoding: chunked\r\n\r\n;
+static const char chunk1[] =
20\r\n\r\n;
+static const char chunk2[] = 18\r\n\r\n;
+static const char chunk3[] =
28\r\n\r\n;
+static const char last_chunk[] = 0\r\n\r\n;
+
+send(c, chunked_header, sizeof(chunked_header)-1, 0);
+send(c, chunk1, sizeof(chunk1)-1, 0);
+WaitForSingleObject(server_event, INFINITE);
+
+send(c, chunk2, sizeof(chunk2)-1, 0);
+WaitForSingleObject(server_event, INFINITE);
+
+send(c, chunk3, sizeof(chunk3)-1, 0);
+WaitForSingleObject(server_event, INFINITE);
+
+send(c, last_chunk, sizeof(last_chunk)-1, 0);
+}
shutdown(c, 2);
closesocket(c);
@@ -2916,6 +2935,130 @@ static void test_no_content(int port)
CloseHandle(hCompleteEvent);
}
+static void test_chunked_stream(int port)
+{
+HINTERNET session, connection, req;
+DWORD res, avail, size;
+BYTE buf[256];
+
+trace(Testing chunked stream...\n);
+
+hCompleteEvent = CreateEvent(NULL, FALSE, FALSE, NULL
Re: [PATCH 2/2] d3d11.idl: Added some missing declarations.
On 09/11/13 14:22, Michael Stefaniuc wrote:
On 09/11/2013 02:11 PM, Henri Verbeet wrote:
On 11 September 2013 12:09, Jacek Caban ja...@codeweavers.com wrote:
+typedef enum D3D11_RESOURCE_MISC_FLAG
+{
+D3D11_RESOURCE_MISC_GENERATE_MIPS= 0x0001,
+D3D11_RESOURCE_MISC_SHARED = 0x0002,
+D3D11_RESOURCE_MISC_TEXTURECUBE = 0x0004,
+D3D11_RESOURCE_MISC_DRAWINDIRECT_ARGS= 0x0010,
+D3D11_RESOURCE_MISC_BUFFER_ALLOW_RAW_VIEWS = 0x0020,
+D3D11_RESOURCE_MISC_BUFFER_STRUCTURED= 0x0040,
+D3D11_RESOURCE_MISC_RESOURCE_CLAMP = 0x0080,
+D3D11_RESOURCE_MISC_SHARED_KEYEDMUTEX= 0x0100,
+D3D11_RESOURCE_MISC_GDI_COMPATIBLE = 0x0200,
+D3D11_RESOURCE_MISC_SHARED_NTHANDLE = 0x0800,
+D3D11_RESOURCE_MISC_RESTRICTED_CONTENT = 0x1000,
+D3D11_RESOURCE_MISC_RESTRICT_SHARED_RESOURCE = 0x2000,
+D3D11_RESOURCE_MISC_RESTRICT_SHARED_RESOURCE_DRIVER = 0x4000,
+D3D11_RESOURCE_MISC_GUARDED = 0x8000
+} D3D11_RESOURCE_MISC_FLAG;
Arguably those should be __MSABI_LONG, although we may not care
enough. We certainly aren't very consistent about it in a lot of other
places either.
I just replaced the long numeric constants with the __MSABI_LONG.
But we don't need those anyway, it's mingw-w64 who needs them for C++
compatibility.
Yes, except we can't do that in IDLs. Looking at this deeper, widl will
produce int constant, no matter what suffix I use. I guess I will add L
suffix to those IDLs so that once widl propagates type properly, it will
deal with __MSABI_LONG.
Jacek
Re: wininet: Don't assume that end of chunk means end of stream.
Hi Hans,
On 09/11/13 13:50, Hans Leidekker wrote:
static DWORD chunked_get_avail_data(data_stream_t *stream, http_request_t
*req)
{
-/* Allow reading only from read buffer */
+chunked_stream_t *chunked_stream = (chunked_stream_t*)stream;
+DWORD res;
+
+if(!chunked_stream-chunk_size || chunked_stream-chunk_size == ~0u) {
+res = start_next_chunk(chunked_stream, req);
+if(res != ERROR_SUCCESS)
+return 0;
+}
start_next_chunk may block and we don't want chunked_get_avail_data to
block.
Jacek
Re: wininet: Don't assume that end of chunk means end of stream.
On 9/11/13 7:09 PM, Hans Leidekker wrote: On Wed, 2013-09-11 at 17:53 +0200, Jacek Caban wrote: Network traces tell me that native also performs a read on the first call to InternetQueryDataAvailable. Are you sure it's not an asynchronous read from a separated thread that is ordered by InternetQueryDataAvailable if no data is available? This was a synchronous request. For synchronous request, if get_avail_data returns 0, we call blocking refill_read_buffer which may start the new chunk. There is no reason for get_avail_data to block. Also note that that's not the interesting thing to test in regards to chunks handling. The tricky part is async request handling. Jacek
Re: wininet: Don't assume that end of chunk means end of stream.
On 09/11/13 17:32, Hans Leidekker wrote:
On Wed, 2013-09-11 at 16:38 +0200, Jacek Caban wrote:
Hi Hans,
On 09/11/13 13:50, Hans Leidekker wrote:
static DWORD chunked_get_avail_data(data_stream_t *stream, http_request_t
*req)
{
-/* Allow reading only from read buffer */
+chunked_stream_t *chunked_stream = (chunked_stream_t*)stream;
+DWORD res;
+
+if(!chunked_stream-chunk_size || chunked_stream-chunk_size == ~0u) {
+res = start_next_chunk(chunked_stream, req);
+if(res != ERROR_SUCCESS)
+return 0;
+}
start_next_chunk may block and we don't want chunked_get_avail_data to
block.
We can't avoid that. If this is the first chunk or if the current chunk has
been consumed we need to read a couple of bytes to find out how much we can
expect.
The first chunk will be received by HTTP_ReceiveRequestData in both
HTTP_HttpSendRequest and HTTP_HttpEndRequest. Subsequent chunks are also
avoidable, just like they didn't block without your patch. We really
can't block in InternetQueryDataAvailable for async request.
Network traces tell me that native also performs a read on the first call to
InternetQueryDataAvailable.
Are you sure it's not an asynchronous read from a separated thread that
is ordered by InternetQueryDataAvailable if no data is available?
Jacek
Re: crypt32 patch review
On 09/05/13 18:53, Juan Lang wrote:
[+wine-devel]
Hi Jacek,
I've added the list so the discussion can take place in public.
On Tue, Sep 3, 2013 at 8:50 AM, Jacek Caban ja...@codeweavers.com
mailto:ja...@codeweavers.com wrote:
I have a patch for crypt32. I'd appreciate your review before I submit
it to Wine. It has high potential to be insecure... This should
fix Wine
bug 28004 as well as some websites that don't send full cert chain in
SSL/TLS handshake, but instead just their own certs. For that we need
two changes. First, we need to look for issuers in global stores
(as in
those that are not passed to CertGetCertificateChain). When I did
that,
I don't really see a security issue here. The global stores are
implicitly trusted, so neglecting to check them was probably an
oversight on my part. Your tests imply something is missing.
My only real question here is whether the test results are in any way
impacted by some unknown caching happening in Windows. Has this chain
been verified prior to running the test? I'm going to guess that the
test bot machines are clean prior to running the tests, but if they
download their test executables from winehq.org http://winehq.org,
that won't be the case.
I'm not sure what's the state of testbot VMs, but I tested it
extensively locally. When tests are ran on a fresh Windows snapshot
(that doesn't have the Rapid SSL cert cached), tests work fine and I can
see HTTP download of the cert in Wireshark. All subsequent runs also
pass, but don't do any HTTP connections.
it came out that it's not enough for intermediate issuers. Those
have to
be downloaded if we have information about their location stored in
validated cert. That's easy using cryptnet. Could you please
review the
attached patch?
+ for(i=0; i info-cAccDescr; i++) {
+if(!strcmp(szOID_PKIX_CA_ISSUERS,
info-rgAccDescr[i].pszAccessMethod)
+ info-rgAccDescr[i].AccessLocation.dwAltNameChoice
== CERT_ALT_NAME_URL) {
You explicitly make use of the AIA extension to find the location of
intermediate certs. This is reasonable, and it gets your test case to
pass, but it won't cover end certs that don't make use of the AIA
extension. As I said above, I wonder whether caching is impacting the
test results. More:
The thing that would be also nice to have as follow up is to cache
downloaded URLs. They are already cached in URL cache, but I believe
that they should be also cached in some place like world collection or
something like that. I'm not sure what would be appropriate. Do
you have
a suggestion?
Yes. I suspect the most relevant bug is actually 27168. 28004 is
probably a dupe of it,
That I do not believe is dupe of 27168, at least according to your
comment 5 in bug 28004 (I didn't debug PartyPoker myself, I just found
it in bugzilla, saw that it's probably the same problem I'm fixing and
verified that it works with my patches).
except that I'm not certain that PartyPoker uses chromium. In brief,
Microsoft's crypt32 appears to cache intermediate certs in a
PCCERT_CONTEXT's hCertStore. What I should be doing in crypt32, but am
not, is to have certs ref count the stores they're in, such that the
last cert to be closed causes its store to be freed, if the store is
closed prior to the cert being freed. This, combined with certificate
chains being cached in Microsoft's implementation, would allow
intermediate certs to be found without explicitly checking the AIA.
You can see how the certificate chain engine ought to support caching
on MSDN:
http://msdn.microsoft.com/en-us/library/windows/desktop/aa377184(v=vs.85).aspx
http://msdn.microsoft.com/en-us/library/windows/desktop/aa377184%28v=vs.85%29.aspx
I never implemented chain caching, as I expected it'd be a performance
optimization and could be addressed at a later time, and never got
back to it.
In addition to bug 27168, you can see how Chromium is making use of
crypt32's certificate caching in
net::X509Certificate::CreateOSCertChainForCert, both its comments and
implementation:
http://src.chromium.org/viewvc/chrome/trunk/src/net/cert/x509_certificate.h
http://src.chromium.org/viewvc/chrome/trunk/src/net/cert/x509_certificate_win.cc
Thanks for the pointers. Looking at 27168 is something I hope to look at
at some point. I hit the problem already when working on schannel and
even in my current patch I had to work around it in one place (and
forgot to put a comment about it...).
Back to your patch: I'm not entirely opposed to the explicit AIA
approach, but I think it's worth at least thinking about other
methods. And, I'd prefer to see the code block more explicitly
separated from the surrounding code, with either a comment or a clear
function name to indicate what's going on. As it is, one has to read
quite a bit of CRYPT_FindIssuer to realize that it's really looking
Re: [PATCH try3] atl110: Added new DLL.
On 09/03/13 13:51, Qian Hong wrote: On Tue, Sep 3, 2013 at 7:42 PM, Jacek Caban ja...@codeweavers.com wrote: Not really, good catch. We should make them consistent. Honestly, I'm not sure which one is better. Both have their problems. Some functions are forwarded, others are not, so having one debug channel would be guarantee that we don't miss some calls while debugging a bug. However, some functions have the same names and are not forwarded, so one debug channel would be ambiguous. I'm open for opinions. How about something like this: atl80.c: -BOOL WINAPI AtlAxWinInit(void) +BOOL WINAPI ATL80_AtlAxWinInit(void) atl80.spec: -42 stdcall AtlAxWinInit() +42 stdcall AtlAxWinInit() ATL80_AtlAxWinInit So we can always use one debug channel for all atlXX dlls, at the same time different exported function with the same name will generate different trace log. Yes, that could work as well. I don't have strong opinion, feel free to submit a patch with solution of your choice. Thanks, Jacek
Re: wininet: Don't start the next chunk if the read is satisfied.
Hi Hans,
On 09/04/13 09:22, Hans Leidekker wrote:
---
dlls/wininet/http.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c
index 1832b4f..521e4aa 100644
--- a/dlls/wininet/http.c
+++ b/dlls/wininet/http.c
@@ -2795,7 +2795,7 @@ static DWORD chunked_read(data_stream_t *stream,
http_request_t *req, BYTE *buf,
chunked_stream_t *chunked_stream = (chunked_stream_t*)stream;
DWORD read_bytes = 0, ret_read = 0, res = ERROR_SUCCESS;
-if(chunked_stream-chunk_size == ~0u) {
+if(!chunked_stream-chunk_size || chunked_stream-chunk_size == ~0u) {
res = start_next_chunk(chunked_stream, req);
if(res != ERROR_SUCCESS)
return res;
@@ -2835,7 +2835,7 @@ static DWORD chunked_read(data_stream_t *stream,
http_request_t *req, BYTE *buf,
chunked_stream-chunk_size -= read_bytes;
size -= read_bytes;
ret_read += read_bytes;
-if(!chunked_stream-chunk_size) {
+if(size !chunked_stream-chunk_size) {
assert(read_mode != READMODE_NOBLOCK);
res = start_next_chunk(chunked_stream, req);
if(res != ERROR_SUCCESS)
This patch breaks two assumptions:
- chunk_size outside chunked_read means the end of the stream
- READMODE_SYNC should read as much data as possible
There is longer story about the way it's implemented the way it is.
Native IE in the past considered zero-length reporting data in
INTERNET_STATUS_REQUEST_COMPLETE to be an error. If you stop reading in
the end of chunk and the next chunk is the last one, then you will send
such report asynchronously. That's why we read the whole chunk only if
we can start reading the next one (and we can't if we don't want to
block). Now I think that the failure could possibly be caused by a
combination of returning zero size and some other bug and the behaviour
that I described could just hide the problem. Commit
851866e22a731141da7e3cbd2550c67c59968959 fixed a problem that could
potentially be the other one.
This means that I'm not entirely sure that we really shouldn't report
zero size data. This is illogical to begin with, but we even have tests
for that:
ok(!on_async, Returned zero size in response to request complete\n);
I think we should try harder to have a test reporting zero size data
(this may be tricky to write one for Wine tests, but if we could just
observe such occasional notifications, that would be enough). And this
would need more testing with native IE.
This all means that intention of the patch may be right, but it needs
more work.
Thanks,
Jacek
Re: wininet: Don't start the next chunk if the read is satisfied.
On 09/04/13 14:38, Hans Leidekker wrote:
On Wed, 2013-09-04 at 13:11 +0200, Jacek Caban wrote:
On 09/04/13 09:22, Hans Leidekker wrote:
---
dlls/wininet/http.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c
index 1832b4f..521e4aa 100644
--- a/dlls/wininet/http.c
+++ b/dlls/wininet/http.c
@@ -2795,7 +2795,7 @@ static DWORD chunked_read(data_stream_t *stream,
http_request_t *req, BYTE *buf,
chunked_stream_t *chunked_stream = (chunked_stream_t*)stream;
DWORD read_bytes = 0, ret_read = 0, res = ERROR_SUCCESS;
-if(chunked_stream-chunk_size == ~0u) {
+if(!chunked_stream-chunk_size || chunked_stream-chunk_size == ~0u) {
res = start_next_chunk(chunked_stream, req);
if(res != ERROR_SUCCESS)
return res;
@@ -2835,7 +2835,7 @@ static DWORD chunked_read(data_stream_t *stream,
http_request_t *req, BYTE *buf,
chunked_stream-chunk_size -= read_bytes;
size -= read_bytes;
ret_read += read_bytes;
-if(!chunked_stream-chunk_size) {
+if(size !chunked_stream-chunk_size) {
assert(read_mode != READMODE_NOBLOCK);
res = start_next_chunk(chunked_stream, req);
if(res != ERROR_SUCCESS)
This patch breaks two assumptions:
- chunk_size outside chunked_read means the end of the stream
Ops, I meant to write chunk_size==0, which means the end of the stream.
That's strange, considering that there could always be another chunk. We can't
know if we're at the end of the stream in chunked mode, so I wonder if we
should
always try the next chunk (and block) if we're being asked to read more than
the
current chunk gives us.
The bug I'm seeing is when the current chunk is exactly the size of the read
request (or has that many bytes left). In that case the current code still
tries
to start the next chunk and potentially blocks.
See chunked_end_of_data. It will return TRUE after the call you
mentioned, no matter if there are more chunks to read.
Obviously we could change chunked_end_of_data and the whole assumptions,
but we need better understanding of desired behaviour here. Consider
this sequence:
- chunked_read reads the whole chunk
- app calls InternetQueryDataAvailable, which has no data buffered, so
it schedules async read
- async read reads zero-size chunk (indicating the end of the stream)
- we call INTERNET_REQUEST_COMPLETE, reporting 0 bytes to read
The last call, as I explained previously, has been problematic in the
past and according to all tests we have, it never happens with navitve
wininet (possibly we simply need more testing to see this happening).
This needs to be investigated, otherwise we risk breaking native IE.
- READMODE_SYNC should read as much data as possible
But no more than asked for? I don't see how my patch breaks this assumption.
You're right, I misread that part, sorry.
Jacek
Re: [2/2] rpcrt4: Clear the connection pool when closing an http connection.
On 09/04/13 15:36, Hans Leidekker wrote: On Wed, 2013-09-04 at 15:28 +0200, Jacek Caban wrote: On 09/04/13 15:01, Hans Leidekker wrote: --- dlls/rpcrt4/rpc_transport.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/rpcrt4/rpc_transport.c b/dlls/rpcrt4/rpc_transport.c index d0857fa..1eb0151 100644 --- a/dlls/rpcrt4/rpc_transport.c +++ b/dlls/rpcrt4/rpc_transport.c @@ -3234,6 +3234,8 @@ static int rpcrt4_ncacn_http_close(RpcConnection *Connection) HeapFree(GetProcessHeap(), 0, httpc-servername); httpc-servername = NULL; + /* don't allow this connection to be reused */ + InternetSetOptionW(NULL, INTERNET_OPTION_SETTINGS_CHANGED, NULL, 0); There must be a better way to do that than removing all existing wininet cached connections to all hosts. Removing INTERNET_FLAG_KEEP_CONNECTION from HttpOpenRequest call would be a good start... We can't do that, it breaks NTLM and Negotiate. Hmm, right. Thinking more about this, I don't see how this connection could be reused. RPC over HTTP shouldn't reach the end of the stream, so it can't be reused once the connection is established, even if client closes the handle. If it is, that's probably a bug in wininet. When did you see this to happen? Jacek
Re: [2/2] rpcrt4: Clear the connection pool when closing an http connection.
On 09/04/13 15:01, Hans Leidekker wrote: --- dlls/rpcrt4/rpc_transport.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/rpcrt4/rpc_transport.c b/dlls/rpcrt4/rpc_transport.c index d0857fa..1eb0151 100644 --- a/dlls/rpcrt4/rpc_transport.c +++ b/dlls/rpcrt4/rpc_transport.c @@ -3234,6 +3234,8 @@ static int rpcrt4_ncacn_http_close(RpcConnection *Connection) HeapFree(GetProcessHeap(), 0, httpc-servername); httpc-servername = NULL; + /* don't allow this connection to be reused */ + InternetSetOptionW(NULL, INTERNET_OPTION_SETTINGS_CHANGED, NULL, 0); There must be a better way to do that than removing all existing wininet cached connections to all hosts. Removing INTERNET_FLAG_KEEP_CONNECTION from HttpOpenRequest call would be a good start... Jacek
Re: [PATCH try3] atl110: Added new DLL.
On 09/03/13 13:28, Qian Hong wrote: On Tue, Sep 3, 2013 at 7:27 PM, Qian Hong fract...@gmail.com wrote: Hi Jacek, we already have a debug channel atl100 for atl100.dll, but we currently use atl for both atl.dll and atl80.dll, do you think it is better to use atl for all, or one debug channel per each dll? Oh, I just found dlls/atl100/atl_ax.c and dlls/atl100/atl.c used different debug channel, is that expected? Not really, good catch. We should make them consistent. Honestly, I'm not sure which one is better. Both have their problems. Some functions are forwarded, others are not, so having one debug channel would be guarantee that we don't miss some calls while debugging a bug. However, some functions have the same names and are not forwarded, so one debug channel would be ambiguous. I'm open for opinions. Thanks, Jacek
Re: mshtml: Use winehq snapshot instead of the main page in tests.
On 08/18/13 13:44, Marvin wrote: 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 http://testbot.winehq.org/JobDetails.pl?Key=26726 Your paranoid android These seem to be caused by different cache handling in IEs. In general, non-cached download would be more interesting, so we should probably enforce that. Jacek
Wine Gecko 2.24-beta1
Hi all,
It's time to do another update of Wine Gecko, so I prepared a new beta
build. It's already on Sourceforge [1]. Until server-side support for
auto install of the package is committed [2], its manual installation is
required [3]. I attached a patch to Wine that is required for the new Gecko.
Anything that uses MSHTML is worth testing. All help with testing is
appreciated!
Cheers,
Jacek
[1] https://sourceforge.net/projects/wine/files/Wine%20Gecko/2.24-beta1/
[2] http://source.winehq.org/patches/data/97881
[3] http://wiki.winehq.org/Gecko
diff --git a/dlls/appwiz.cpl/addons.c b/dlls/appwiz.cpl/addons.c
index be6fb6d..c5070dc 100644
--- a/dlls/appwiz.cpl/addons.c
+++ b/dlls/appwiz.cpl/addons.c
@@ -53,14 +53,14 @@
WINE_DEFAULT_DEBUG_CHANNEL(appwizcpl);
-#define GECKO_VERSION 2.21
+#define GECKO_VERSION 2.24-beta1
#ifdef __i386__
#define ARCH_STRING x86
-#define GECKO_SHA a514fc4d53783a586c7880a676c415695fe934a3
+#define GECKO_SHA 659cce1a0e2f4c6768aedf7bcbb4072e8a465c43
#elif defined(__x86_64__)
#define ARCH_STRING x86_64
-#define GECKO_SHA c6f249ff2c6eb7dfe423ef246aba54e1a3b26934
+#define GECKO_SHA 31f182f0e2003537aadb27328d3bcd8f3598b722
#else
#define ARCH_STRING
#define GECKO_SHA ???
diff --git a/dlls/mshtml/htmlelem.c b/dlls/mshtml/htmlelem.c
index db4b8e7..73e8f55 100644
--- a/dlls/mshtml/htmlelem.c
+++ b/dlls/mshtml/htmlelem.c
@@ -1728,8 +1728,8 @@ static HRESULT HTMLElement_invoke(DispatchEx *dispex,
DISPID id, LCID lcid,
static HRESULT HTMLElement_populate_props(DispatchEx *dispex)
{
HTMLElement *This = impl_from_DispatchEx(dispex);
-nsIDOMNamedNodeMap *attrs;
-nsIDOMNode *node;
+nsIDOMMozNamedAttrMap *attrs;
+nsIDOMAttr *attr;
nsAString nsstr;
const PRUnichar *str;
BSTR name;
@@ -1747,40 +1747,40 @@ static HRESULT HTMLElement_populate_props(DispatchEx
*dispex)
if(NS_FAILED(nsres))
return E_FAIL;
-nsres = nsIDOMNamedNodeMap_GetLength(attrs, len);
+nsres = nsIDOMMozNamedAttrMap_GetLength(attrs, len);
if(NS_FAILED(nsres)) {
-nsIDOMNamedNodeMap_Release(attrs);
+nsIDOMMozNamedAttrMap_Release(attrs);
return E_FAIL;
}
nsAString_Init(nsstr, NULL);
for(i=0; ilen; i++) {
-nsres = nsIDOMNamedNodeMap_Item(attrs, i, node);
+nsres = nsIDOMMozNamedAttrMap_Item(attrs, i, attr);
if(NS_FAILED(nsres))
continue;
-nsres = nsIDOMNode_GetNodeName(node, nsstr);
+nsres = nsIDOMAttr_GetNodeName(attr, nsstr);
if(NS_FAILED(nsres)) {
-nsIDOMNode_Release(node);
+nsIDOMAttr_Release(attr);
continue;
}
nsAString_GetData(nsstr, str);
name = SysAllocString(str);
if(!name) {
-nsIDOMNode_Release(node);
+nsIDOMAttr_Release(attr);
continue;
}
hres = IDispatchEx_GetDispID(dispex-IDispatchEx_iface, name,
fdexNameCaseInsensitive, id);
if(hres != DISP_E_UNKNOWNNAME) {
-nsIDOMNode_Release(node);
+nsIDOMAttr_Release(attr);
SysFreeString(name);
continue;
}
-nsres = nsIDOMNode_GetNodeValue(node, nsstr);
-nsIDOMNode_Release(node);
+nsres = nsIDOMAttr_GetNodeValue(attr, nsstr);
+nsIDOMAttr_Release(attr);
if(NS_FAILED(nsres)) {
SysFreeString(name);
continue;
@@ -1803,7 +1803,7 @@ static HRESULT HTMLElement_populate_props(DispatchEx
*dispex)
}
nsAString_Finish(nsstr);
-nsIDOMNamedNodeMap_Release(attrs);
+nsIDOMMozNamedAttrMap_Release(attrs);
return S_OK;
}
diff --git a/dlls/mshtml/htmlwindow.c b/dlls/mshtml/htmlwindow.c
index 5c40f41..65161fd 100644
--- a/dlls/mshtml/htmlwindow.c
+++ b/dlls/mshtml/htmlwindow.c
@@ -38,6 +38,7 @@
#include mshtml_private.h
#include htmlevent.h
#include htmlscript.h
+#include pluginhost.h
#include binding.h
#include resource.h
@@ -114,8 +115,12 @@ static void detach_inner_window(HTMLInnerWindow *window)
if(outer_window outer_window-doc_obj outer_window ==
outer_window-doc_obj-basedoc.window)
window-doc-basedoc.cp_container.forward_container = NULL;
-if(window-doc)
+if(window-doc) {
detach_events(window-doc);
+while(!list_empty(window-doc-plugin_hosts))
+
detach_plugin_host(LIST_ENTRY(list_head(window-doc-plugin_hosts),
PluginHost, entry));
+}
+
abort_window_bindings(window);
remove_target_tasks(window-task_magic);
release_script_hosts(window);
diff --git a/dlls/mshtml/nsembed.c b/dlls/mshtml/nsembed.c
index 4e72464..7fa13e5 100644
--- a/dlls/mshtml/nsembed.c
+++ b/dlls/mshtml/nsembed.c
@@ -486,7 +486,7 @@ static BOOL load_xul(const PRUnichar *gre_path)
}
#define NS_DLSYM(func) \
-func = (void *)GetProcAddress(xul_handle, #func _P); \
+func = (void *)GetProcAddress(xul_handle, #func); \
if(!func) \
ERR(Could not
Wine Gecko repo
Hi, A few months ago, SourceForge did some deep changes in their Git (and not only) repo hosting. While doing it, the location of repos has changed (which is extremely stupid, if you ask me). The problem is that while doing the update, they also switched to their own Web interface for git. And this shiny new interface doesn't work at all for Wine Gecko repo (wine-gecko.git probably too big and complicated). See [1], it claims to be analyzing the repo, but it has been like that since the switch was done. I contacted them [2] and they are aware of the problem, but it's still not fixed. I already got some questions about outdated sources. Also, our Wiki points to this Git in a few places. I never updated those links because I was waiting for the new repo to work. I think it's time to move the repo. The question is, where should it go? - Github seems to be the choice for most project currently and it's already used by Wine Mono. - Reconsider source.winehq.org. This was not chosen due to account management overhead, but that's the only way to avoid problems similar to recent Sourceforge one (Github nor anyone else is going to give us any guarantees) - Other ideas? I'd appreciate opinions. Thanks, Jacek [1] https://sourceforge.net/p/wine/wine-gecko/ci/master/tree/ [2] https://sourceforge.net/p/forge/site-support/4299/
Re: [PATCH 01/11] widl: New option --rt for enabling rt's specific language extensions.
On 08/08/13 19:56, Kai Tietz wrote: 2013/8/8 André Hentschel n...@dawncrow.de: Am 08.08.2013 17:51, schrieb Kai Tietz: Hi, this patch adds new --rt option to widl. By it you can control, if RT's IDL-language-extension(?s) getting active. This initial patch just adds the option-scanning and the global flag-variable to widl. Ok for apply? I'd say no, you are adding dead code, so you might want to merge it with 2/11 to avoid that. Well, it isn't dead-code. Patch contains one complete thing. It introduces an new switch. I separate this switch out, as it can be reviewed absolutely independently to the actual namespace lexer/parser thing (which is the first user of this switch). Jacek asked me to break things down in small logical pieces, so I did. Yeah, deciding on how much to split patches is sometimes tricky. This patch does not introduce dead code (but rather dead command like argument), so I think it's fine. This needs man page update, but that may be done as a follow up. Jacek
Re: riched20: ITextDocument stubs for ITextServices (retry)
On 07/29/13 17:55, Caibin Chen wrote:
+struct tagReTxtDoc {
+ IUnknown *outerObj;
+ ME_TextEditor *editor;
+ ITextDocument iTextDocumentIface;
+};
Do you really need a separated structure for that? Wouldn't adding
ITextDocument implementation to ITextServicesImpl be enough?
This structure will be used in both ITexstServicesImpl and
IRichEditOleImpl(richole.c). For now IRichEditOleImpl has its own
implementation(stub) of ITextDocument and ITextSelection. A stub of
ITextSelection is needed before removing IRichEditOleImpl's own
implementation and use the separate one.
There are already several patches that are ready for review once this
patch get submitted.
I see, assuming that substantial part of the implementation may really
be shared between those objects, that makes sense. Bellow is review of
the remaining part of the patch:
+struct tagReTxtDoc {
+ IUnknown *outerObj;
+ ME_TextEditor *editor;
+ ITextDocument iTextDocumentIface;
+};
Please follow http://wiki.winehq.org/COMGuideline about naming
convention (*_iface names).
+ITextDocument *ReTxtDoc_get_ITextDocument(ReTxtDoc *document)
+{
+ TRACE((%p)\n, document);
+ return document-iTextDocumentIface;
+}
Full declaration of ReTxtDoc in a header (so you don't need such helper) would
be cleaner, IMO.
new file mode 100644
index 000..02579b2
--- /dev/null
+++ b/dlls/riched20/txtdoc.h
How about using an existing header?
Thanks,
Jacek
Re: [1/2] msxml3: Implement IMXAttributes_removeAttributes()
Hi Nikolay, On 07/30/13 12:42, Nikolay Sivov wrote: +dst = This-attr[index]; +src = This-attr[index+1]; + +memmove(dst, src, This-length-index-1); *sizeof(*dst) Cheers, Jacek
Re: riched20: ITextDocument stubs for ITextServices (retry)
Hi Caibin,
On 07/29/13 01:52, Caibin Chen wrote:
Hi,
It has been 2 weeks since I sent this patch[1]. It's still new status
in the patches list. I'm working on following patches based on it. I
appreciate your reviews and will work on the patch to get it
submitted.
You sent the patch during code freeze, when it couldn't be committed, so
it probably didn't get enough attention. Now is the right time to resubmit.
+/* COM aggregation on ITextDocument */
+hr = IUnknown_QueryInterface(unk_obj.inner_unk, IID_ITextDocument,
(void**)textdoc);
+ok(hr == S_OK, QueryInterface for IID_ITextDocument failed: %08x\n, hr);
+refcount = ITextDocument_AddRef(textdoc);
+ok(refcount == unk_obj.ref, CreateTextServices just pretends to support
COM aggregation\n);
+refcount = ITextDocument_Release(textdoc);
+ok(refcount == unk_obj.ref, CreateTextServices just pretends to support
COM aggregation\n);
+refcount = ITextDocument_Release(textdoc);
It doesn't really make sense to test COM aggregation on every interface
implemented by an object. The valuable part of the test is that
ITextDocument should be supported, but there is probably a better place
to put this test.
+
+struct tagReTxtDoc {
+ IUnknown *outerObj;
+ ME_TextEditor *editor;
+ ITextDocument iTextDocumentIface;
+};
Do you really need a separated structure for that? Wouldn't adding
ITextDocument implementation to ITextServicesImpl be enough?
Thanks,
Jacek
Re: widl: Do not generate C structure for empty interfaces. (try 2)
On 06/30/13 23:14, Thomas Faber wrote:
On 2013-06-27 13:33, Jacek Caban wrote:
That
said, if such interfaces are just obscure special case, I would say we
shouldn't care. We may easily avoid them (I just sent a patch avoiding
it in mshtml).
Ah, I hadn't even considered changing the interface. Seemed like it was
something mandated by Mozilla's use of it.
This indeed seems to be the best solution to the immediate problem of
mshtml not building with MSVC.
Thanks!
In general, Mozilla uses their own variant of COM, called XPCOM. It
happens to be similar enough to MS variant of COM that we may use widl
to generate its binary compatible headers, but if there is any
incompatibility, fixing it in nsiface.idl is preferred over hacking widl.
If you want a better solution to avoid such problems in the future, I
would suggest adding an error, like midl does. This will, however,
require some more changes in Wine. At least MSHTML already has an
interface identical to IUnknown (nsISupports), which would cause the
error. This should be solvable with some tricks.
That sounds like the smart thing to do. I've attached the simplest
version that comes to mind.
The interfaces I needed to exempt were IUnknown (naturally),
ID3DInclude (which PSDK seems to define via cpp_quote), and nsISupports
as you already mentioned.
Midl also seems to have an if not inside a library condition, which
I'll try to add as well. I'd have liked to use is_object in widl, but
that also triggers on the odl attribute.
I'll send it to -patches once I'm satisfied with it, but I'm pretty
fond of the simple approach already. Let me know if you have any
comments.
I don't think we want to maintain such a list of special interfaces in
widl sources. IUnknown is a special case in many aspects, so that's
fine. ID3DInclude is something that definitely shouldn't be there. It's
also small enough that cpp_quote should be fine for this. nsISupports
is, as I said earlier, something that would be better worked around in
nsiface.idl. The attached patch does that, although now there will
probably be duplicated UUID error on midl. That's also something we
could work around with some preprocessor tricks...
If this would be generic enough problem, we could add some widl
extension, like a special attribute, but your findings show that it's
too rare to mitigate that.
Thanks,
Jacek
diff --git a/dlls/mshtml/nsiface.idl b/dlls/mshtml/nsiface.idl
index fcdf6f1..3bbcd46 100644
--- a/dlls/mshtml/nsiface.idl
+++ b/dlls/mshtml/nsiface.idl
@@ -26,6 +26,7 @@
cpp_quote(#define GECKO_VERSION \2.21\)
cpp_quote(#define GECKO_VERSION_STRING \Wine Gecko \ GECKO_VERSION)
+import unknwn.idl;
import wtypes.idl;
cpp_quote(#ifdef WINE_NO_UNICODE_MACROS)
@@ -114,12 +115,8 @@ interface IMoniker;
uuid(---c000-0046),
local
]
-interface nsISupports
-{
-nsresult QueryInterface(nsIIDRef riid, void **result);
-nsrefcnt AddRef();
-nsrefcnt Release();
-}
+interface nsISupports : IUnknown
+{}
/* Currently we don't need a full declaration of these interfaces */
typedef nsISupports nsISHistory;
Re: Duplicate UUID in oleaut32:tmarshal test
On 06/30/13 23:29, Thomas Faber wrote: I just tried running midl on dlls/oleaut32/tests/tmarshal.idl and it complained: tmarshal.idl(83) : error MIDL2270 : duplicate UUID. Same as : ItestDual [ Interface 'ITestSecondDisp' ] This doesn't seem like it's intentional. But just giving ITestSecondDisp a new uuid causes the following test failures on Windows: tmarshal.c:1861: Test failed: external_connections = 2 tmarshal.c:1864: Test failed: external_connections = 2 tmarshal.c:192: Test failed: fLastReleaseCloses = 1 I could try to figure this out, but it's probably better if someone familiar with the subject could have a look instead. (also, assuming this is a bug in the test, we might want to add that check to widl, it seems pretty useful ;]) Thanks for the report. This was indeed a stupid mistake and after fixing it, the behaviour is more logical. I will send a patch fixing tests. Fixing todo_wine may wait until after code freeze. Current Wine behaviour shouldn't cause problems. Thanks, Jacek
Re: widl: Do not generate C structure for empty interfaces. (try 2)
Hi Thomas, I tested what midl does in this case and I think the solution should be different. When I tried to compile an empty interface, I got an error saying that '[object] interface must derive from another [object] interface'. That requirement is mostly enough to avoid empty vtbl. I was wondering how IUnknown is handled by midl (since it doesn't derive from any other iface). It seems like it's a simple name-based special case. When I renamed the interface to IUnknown, it compiled fine and generated vtbl the same as current widl (that is an empty struct). That said, if such interfaces are just obscure special case, I would say we shouldn't care. We may easily avoid them (I just sent a patch avoiding it in mshtml). If you want a better solution to avoid such problems in the future, I would suggest adding an error, like midl does. This will, however, require some more changes in Wine. At least MSHTML already has an interface identical to IUnknown (nsISupports), which would cause the error. This should be solvable with some tricks. Thanks, Jacek
Re: [PATCH 1/2]vbscript: Implemented builtin function CInt
Hi Zhan, On 05/17/13 16:53, Zhan Jianyu wrote: This patch is built based on Jaceks's newly-submitted patch: commit 48a862306288d69d3623d5ecdc0f5a5a40addafe ( vbscript: Round half to even in to_int) Thanks, Jacek. :-) With his patch, CInt could be implementd in a clean way, without any introduced helper function. This patch also add tests for any corner cases, and it has passed testbot's testing. --- dlls/vbscript/global.c | 14 -- dlls/vbscript/tests/api.vbs | 23 +++ 2 files changed, 35 insertions(+), 2 deletions(-) As you may check here: http://source.winehq.org/patches/ your patch is marked as 'Apply failure'. Did you prepare the patch against recent Wine Git? Please rebase and resend the patch. Cheers, Jacek
Re: [PATCH 1/2]vbscript: Implemented builtin function CInt(try 3)
Hi Zhan,
It's better, thanks, but still needs more work.
On 04/25/13 23:37, Zhan Jianyu wrote:
}
+
+
While we're at this, please put more attention into whitespace changes,
esp. useless ones.
static HRESULT to_double(VARIANT *v, double *ret)
{
switch(V_VT(v)) {
@@ -217,6 +221,27 @@ static HRESULT to_string(VARIANT *v, BSTR *ret)
return S_OK;
}
+
+static HRESULT banker_rounding(double dbval, int *result_val)
+{
+ double frac_part;
+ double int_part;
+
+ *result_val = round(dbval);
This has two problems:
- you don't handle values that are out of int range by assigning double
value to int
- round is not portable. As I mentioned you before, Wine needs to be C89
compatible. If we need some features that are not part of C89, we need
to be careful about that and it often requires things like configure
checks. In case of round, replacing it with floor(x+0.5) does the trick
pretty well.
Cheers,
Jacek
Re: [PATCH 1/2]vbscript: Implemented builtin function CInt(try 3)
On 04/26/13 12:02, Henri Verbeet wrote: On 26 April 2013 10:27, Jacek Caban ja...@codeweavers.com wrote: - round is not portable. As I mentioned you before, Wine needs to be C89 compatible. If we need some features that are not part of C89, we need to be careful about that and it often requires things like configure checks. In case of round, replacing it with floor(x+0.5) does the trick pretty well. But that's going to do something different for negative values. That will be different only for numbers with fractional part equal to 0.5, which we have to handle separately using both round and floor anyway. Jacek
Re: msvcrt.h: Use inline function to forward hypot to _hypot.
On 04/26/13 14:46, Alexandre Julliard wrote: Jacek Caban ja...@codeweavers.com writes: This fixes compiling msvcp* DLLs with recent mingw-w64 trunk version. Previous versions had a separated library that contained forwards from underscored functions to non-underscored, which I believe made using hypot work. Those were currently moved directly to msvcr* import libs, which we don't use in favour of our our own ones. Don't you need to do that for the other functions too? With just this patch, the compilation works fine for me. There were not that many functions affected by mingw-w64 change. Note that most msvcrt functions are exported without underscore prefix. AFAICS, MSVC has such inline wrapper in math.h only for hypot and hypotf. I checked why hypodf was not affected so far and it seems like it's just because it uses different way of forwarding in mingw-w64 crt, so that's also worth changing in our math.h to be safe for the future changes. Jacek
Re: msvcrt.h: Use inline function to forward hypot to _hypot.
On 04/26/13 16:18, Alexandre Julliard wrote: Jacek Caban ja...@codeweavers.com writes: On 04/26/13 14:46, Alexandre Julliard wrote: Jacek Caban ja...@codeweavers.com writes: This fixes compiling msvcp* DLLs with recent mingw-w64 trunk version. Previous versions had a separated library that contained forwards from underscored functions to non-underscored, which I believe made using hypot work. Those were currently moved directly to msvcr* import libs, which we don't use in favour of our our own ones. Don't you need to do that for the other functions too? With just this patch, the compilation works fine for me. There were not that many functions affected by mingw-w64 change. Note that most msvcrt functions are exported without underscore prefix. AFAICS, MSVC has such inline wrapper in math.h only for hypot and hypotf. I checked why hypodf was not affected so far and it seems like it's just because it uses different way of forwarding in mingw-w64 crt, so that's also worth changing in our math.h to be safe for the future changes. It looks to me like j0/j1/jn etc. would need it too. We don't use them outside msvcrt, AFAICS. But yeah, we could have it for sake of completeness. Jacek
Wine Gecko 2.21-beta1
Hi all,
As some of you might have noticed, the final Wine Gecko 2.20 was never
been released, although it was scheduled for about three weeks ago. This
is mostly due to my lack of time, combined with the fact that part of
that time I was busy with some Gecko work that would be nice to have in
Wine. Being 3 weeks late means that we also have 3 weeks until the next
Firefox release, so we may just wait a bit longer and do another release
based on newer upstream code base. This way I may use the new beta to
include some of the changes that I did lately, which would be too risky
to go straight to release. Those new improvements contain, among others,
a fix for Adobe CS4 products installers.
In this beta, the testing is easier than ever. Our auto installer knows
about the beta which, combined with the fact that we cache downloaded
packages now, means that the attached Wine patch is all you need to
comfortably test the new package. Just apply the patch you the Wine tree
and everything should work. All tests are appreciated.
Thanks,
Jacek
diff --git a/dlls/appwiz.cpl/addons.c b/dlls/appwiz.cpl/addons.c
index 2a47b84..a3bfd0b 100644
--- a/dlls/appwiz.cpl/addons.c
+++ b/dlls/appwiz.cpl/addons.c
@@ -53,14 +53,14 @@
WINE_DEFAULT_DEBUG_CHANNEL(appwizcpl);
-#define GECKO_VERSION 1.9
+#define GECKO_VERSION 2.21-beta1
#ifdef __i386__
#define ARCH_STRING x86
-#define GECKO_SHA d2553224848a926eacfa8685662ff1d7e8be2428
+#define GECKO_SHA 246672bf7b5c6fb896865f2e2abadf5e16bdcaea
#elif defined(__x86_64__)
#define ARCH_STRING x86_64
-#define GECKO_SHA c7cd0994f89dd15b36ce8dacaa33d0ec47c407d1
+#define GECKO_SHA 389aea6f0bc3db77ad1ed0645ea3a4885a5df04f
#else
#define ARCH_STRING
#define GECKO_SHA ???
diff --git a/dlls/mshtml/htmlelem2.c b/dlls/mshtml/htmlelem2.c
index 24aaba5..516a3f5 100644
--- a/dlls/mshtml/htmlelem2.c
+++ b/dlls/mshtml/htmlelem2.c
@@ -1253,22 +1253,22 @@ static HRESULT WINAPI
HTMLElement2_getElementsByTagName(IHTMLElement2 *iface, BS
IHTMLElementCollection
**pelColl)
{
HTMLElement *This = impl_from_IHTMLElement2(iface);
-nsIDOMNodeList *nslist;
+nsIDOMHTMLCollection *nscol;
nsAString tag_str;
nsresult nsres;
TRACE((%p)-(%s %p)\n, This, debugstr_w(v), pelColl);
nsAString_InitDepend(tag_str, v);
-nsres = nsIDOMHTMLElement_GetElementsByTagName(This-nselem, tag_str,
nslist);
+nsres = nsIDOMHTMLElement_GetElementsByTagName(This-nselem, tag_str,
nscol);
nsAString_Finish(tag_str);
if(NS_FAILED(nsres)) {
ERR(GetElementByTagName failed: %08x\n, nsres);
return E_FAIL;
}
-*pelColl = create_collection_from_nodelist(This-node.doc, nslist);
-nsIDOMNodeList_Release(nslist);
+*pelColl = create_collection_from_htmlcol(This-node.doc, nscol);
+nsIDOMHTMLCollection_Release(nscol);
return S_OK;
}
diff --git a/dlls/mshtml/nsiface.idl b/dlls/mshtml/nsiface.idl
index 368a154..18653e6 100644
--- a/dlls/mshtml/nsiface.idl
+++ b/dlls/mshtml/nsiface.idl
@@ -23,7 +23,7 @@
* compatible with XPCOM, usable in C code.
*/
-cpp_quote(#define GECKO_VERSION \1.9\)
+cpp_quote(#define GECKO_VERSION \2.21-beta1\)
cpp_quote(#define GECKO_VERSION_STRING \Wine Gecko \ GECKO_VERSION)
import wtypes.idl;
@@ -179,10 +179,7 @@ typedef nsISupports nsIDOMHistory;
typedef nsISupports nsIDOMNavigator;
typedef nsISupports nsIDOMMediaQueryList;
typedef nsISupports nsIDOMScreen;
-typedef nsISupports nsIDOMCrypto;
-typedef nsISupports nsIDOMPkcs11;
typedef nsISupports nsIAnimationFrameListener;
-typedef nsISupports nsIDOMMozURLProperty;
typedef nsISupports nsIDOMStorageList;
typedef nsISupports nsILocalFile;
typedef nsISupports nsIDOMHTMLMenuElement;
@@ -195,6 +192,10 @@ typedef nsISupports nsIDOMBlob;
typedef nsISupports nsIPrivacyTransitionObserver;
typedef nsISupports nsIDOMHTMLPropertiesCollection;
typedef nsISupports mozIDOMApplication;
+typedef nsISupports nsILoadGroupConnectionInfo;
+typedef nsISupports nsIDOMCrypto;
+typedef nsISupports nsIDOMPkcs11;
+typedef nsISupports nsIDocShellTreeOwner;
typedef void *JSContext;
typedef void *JSObject;
@@ -226,7 +227,7 @@ interface nsIFactory : nsISupports
[
object,
-uuid(59e7e77a-38e4-11d4-8cf5-0060b0fc14a3),
+uuid(6aef11c4-8615-44a6-9711-98f43805693d),
local
]
interface nsIMemory : nsISupports
@@ -236,6 +237,7 @@ interface nsIMemory : nsISupports
void Free(void *_ptr);
nsresult HeapMinimize(bool immediate);
nsresult IsLowMemory(bool *_retval);
+nsresult IsLowMemoryPlatform(bool *_retval);
}
[
@@ -521,7 +523,7 @@ interface nsIStreamListener : nsIRequestObserver
[
object,
-uuid(3de0a31c-feaf-400f-9f1e-4ef71f8b20cc),
+uuid(19501006-46e3-4634-b97d-26eff894b4d3),
local
]
interface nsILoadGroup : nsIRequest
@@ -536,11 +538,12 @@ interface nsILoadGroup : nsIRequest
nsresult GetActiveCount(uint32_t *aActiveCount);
nsresult
Re: oledb32: Implement DataConvert DBTYPE_BOOL-VARIANT
On 04/24/13 10:41, Alistair Leslie-Hughes wrote: +case DBTYPE_BOOL: +V_VT(v) = VT_BOOL; +V_BOOL(v) = *(BOOL*)src; You're mixing BOOL and VARIANT_BOOL here. This can't work, they even have a different size. From your test I guess that you meant to cast src to VARIANT_BOOL*. Cheers, Jacek
Re: Clang static analyzer results / wine-1.5.28-66-g6899279
On 04/24/13 11:35, Frédéric Delanoy wrote: On Wed, Apr 24, 2013 at 7:40 AM, Austin English austinengl...@gmail.com mailto:austinengl...@gmail.com wrote: On Fri, Apr 19, 2013 at 1:52 AM, Michael Stefaniuc mstef...@redhat.com mailto:mstef...@redhat.com wrote: On 04/19/2013 12:43 AM, Austin English wrote: With wine-1.5.20 and clang 3.2, the test suite is in the same state on my Fedora 18 machine as gcc-4.7.2 llvm version: git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@179768 91177308-0d34-0410-b5e6-96231b3b80d8 clang version: git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179771 91177308-0d34-0410-b5e6-96231b3b80d8 ./configure --disable-tests was used to reduce noise Static analyse of the tests is important as well. Over the years I found bad tests that didn't test what they were supposed to test. And even cases where the Wine code was bad due to that as it passed the tests. bye michael That was suggested by Jacek, see http://www.winehq.org/pipermail/wine-devel/2012-January/094059.html Sure, but Michael has a good point here IMHO. Yeah, if it finds a real problem, then I agree. The problem with reports from tests is that there are tons of things like error handling that we don't intend to 'fix', so they do (and always will) make it harder to find similar real bugs in the rest of code base. Plus we're never going to be anywhere close to 0 problems this way ;) Although, one can sort/filter by file, or if it's really that annoying, a quick script removing tr elements containing /tests should be easy enough. Well... that's far from perfect, but could work. Cheers, Jacek
Re: Application for GSoC 2013
On 04/17/13 18:38, larmbr zhan wrote: Hi all, I'm a junior student from a non-famous college, I apply to parcitipate in GSoC , to help do some vbscript module implementation. I've just submitted a patchset on this. No worries, no collage name can give you as much as a patch. That's a good start and I think vbscript improvement can be a good GSoC project. Cheers, Jacek
Re: [PATCH 2/3]vbscript: Implemented builtin function CInt
On 04/17/13 18:17, larmbr zhan wrote:
+static HRESULT to_int_banker_rounding(VARIANT *v, int *ret)
+{
I wonder if this conversion is used in any other place as well. I think
this does not need a helper function until we find other use cases as
well, esp. since you could simplify that to:
if(V_VT(v) == VT_R8) {
//... your conversion
}else {
hres = to_int(v);
}
+case VT_R8: {
+double n = rint(V_R8(v));
rint is not portable. Please avoid using it.
@@ -424,4 +424,22 @@ Call ok(getVT(vbYes) = VT_I2, getVT(vbYes) =
getVT(vbYes))
Call ok(vbNo = 7, vbNo = vbNo)
Call ok(getVT(vbNo) = VT_I2, getVT(vbNo) = getVT(vbNo))
+Call ok(CInt(-36.75) = -37, CInt(-36.75) = CInt(-36.75))
+Call ok(CInt(-36.50) = -36, CInt(-36.50) = CInt(-36.50))
+Call ok(CInt(-36.25) = -36, CInt(-36.25) = CInt(-36.25))
Please add some tests using getVT to verify underlying types.
Thanks,
Jacek
Re: [PATCH 3/3]vbscript: Implemented builtin function CBool
On 04/17/13 18:18, larmbr zhan wrote:
+double tmp;
+WCHAR trueW[] = {'T','r','u','e','\0'};
+WCHAR falseW[] = {'F','a','l','s','e','\0'};
+
+TRACE(%s\n, debugstr_variant(arg));
+
+assert(args_cnt == 1);
+
+switch(V_VT(arg)) {
+case VT_I2:
+tmp = V_I2(arg);
+break;
+case VT_I4:
+tmp = V_I4(arg);
+break;
I don't think converting to double is a good idea here. Why don't you do
conversions directly to BOOL for all cases?
+case VT_R4:
+tmp = V_R4(arg);
+break;
+case VT_R8:
+tmp = V_R8(arg);
+break;
+default:
+ERR(Not a numeric vaule: %s\n, debugstr_variant(arg));
+return E_FAIL;
+}
+
+if (tmp 0.0 || tmp 0.0)
+str = SysAllocString(trueW);
+else
+str = SysAllocString(falseW);
+
+
+return return_bstr(res, str);
Are you sure you want to return string type here? I'd expect a bool
value (of type VT_BOOL) here...
}
static HRESULT Global_CByte(vbdisp_t *This, VARIANT *arg, unsigned args_cnt,
VARIANT *res)
diff --git a/dlls/vbscript/tests/api.vbs b/dlls/vbscript/tests/api.vbs
index 6bd4065..9b25e87 100644
--- a/dlls/vbscript/tests/api.vbs
+++ b/dlls/vbscript/tests/api.vbs
@@ -442,4 +442,9 @@ Call ok(CInt(36.50) = 36, CInt(36.50) = CInt(36.50))
Call ok(CInt(36.75) = 37, CInt(36.75) = CInt(36.75))
Call ok(CInt(300) = 300, CInt(300) = CInt(300))
Call ok(CInt(-300) = -300, CInt(-300) = CInt(-300))
+
+Call ok(CBool(5) = True, CBool(5) = CBool(5))
+Call ok(CBool(0) = False, CBool(0) = CBool(0))
+Call ok(CBool(-5) = True, CBool(-5) = CBool(-5))
...and that's why getVT(...) test would be nice here.
Thanks,
Jacek
Re: Application for GSoC 2013
On 04/18/13 12:06, larmbr zhan wrote: You're going to have to be (much) more specific with your application, what modules do you plan to implement, what applications does this help (if you know of any), etc. Actually I am a huge fan of underlying stuff like OS, programming language internals or so. So I think vbscript implementation is right up my alley. Yes, vbscript sounds right then. It's a nice language to learn from others' (MS in this case) mistakes :) And For specific field in this component, I would said to fill some blank of vbscript component, since this may block some user applications . e.g. http://bugs.winehq.org/show_bug.cgi?id=31025 I think picking up the implementation of array support for vbscript would be a challenge and a significant task. Well, support of arrays requires touching about every part of the engine, so it's not something for the first patch. It's better to start with simpler parts, like you already do. Cheers, Jacek
Re: [PATCH 6/6] ieframe: Added DISPID_WINDOWCLOSING tests.
On 04/15/13 18:08, Marvin wrote: 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 http://testbot.winehq.org/JobDetails.pl?Key=25086 Your paranoid android. The patch was rejected due to those failures, but: === W2KPROSP4 (32 bit webbrowser) === webbrowser.c:2631: Test failed: expected GetOverridesKeyPath webbrowser.c:2636: Test failed: expected Invoke_SETSECURELOCKICON webbrowser.c:2637: Test failed: expected Invoke_FILEDOWNLOAD webbrowser.c:3076: Test failed: doc_disp == NULL webbrowser: unhandled exception c005 at 00402F21 This one is TestBot's fault. The whole test should be skipped on that machine (and is skipped by WineTest). === WVISTAX64 (64 bit webbrowser) === webbrowser.c:978: Test failed: unexpected dispIdMember 271 webbrowser.c:1604: Test failed: unexpected call TranslateUrl webbrowser.c:454: Test failed: unexpected nCmdID 14 Timeout This one seems like rare random unrelated failure. Here is a clean run of the same patch: https://testbot.winehq.org/JobDetails.pl?Key=25094 I will resend the patch. Cheers, Jacek
Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.
Hi Ken, On 04/10/13 16:16, Ken Thomases wrote: Hi Jacek, On Apr 10, 2013, at 4:24 AM, Jacek Caban wrote: On 3/28/13 8:31 PM, Ken Thomases wrote: Mac OS X 10.8 introduced support for TLS 1.1 and 1.2. Can someone with Mac OS X 10.8 test the attached patch for me, please. All I need is to verify that it compiles and when running dlls/secur32/tests/secur32_test.exe.so schannel, TLS 1.1 and TLS 1.2 are listed as supported protocol. Thanks for doing this. I don't have a 10.8 build system handy, myself, or I would have done it or would test what you've done. However, Apple's guidance on using weak linking says that you must explicitly compare against NULL. They don't quite say that testing the symbol as a standalone boolean expression won't work, but they do say that using negation (the ! operator) isn't sufficient. https://developer.apple.com/library/mac/documentation/developertools/conceptual/cross_development/Using/using.html#//apple_ref/doc/uid/20002000-SW4 I'm not sure what the rationale is, but I think there's a peculiarity of what the compiler thinks it knows versus what the linker actually knows. Basically, the compiler figures there's a symbol that will be resolved (or cause a link error), so the check can be optimized away somehow. I think. Oh, such restriction seem to indicate a broken design of this weak linking... Following this is fine, through, but perhaps we should use nil to be sure that NULL override in winnt.h won't confuse the heuristic? Other than that, the patch looks good to me. Thanks for the review. Cheers, Jacek
Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.
On 3/28/13 8:31 PM, Ken Thomases wrote:
Mac OS X 10.8 introduced support for TLS 1.1 and 1.2.
Can someone with Mac OS X 10.8 test the attached patch for me, please.
All I need is to verify that it compiles and when running
dlls/secur32/tests/secur32_test.exe.so schannel, TLS 1.1 and TLS 1.2 are
listed as supported protocol.
Thanks,
Jacek
commit 78f9768f8d6759af1df99c4b67b8fd6a93369da4
Author: Jacek Caban ja...@codeweavers.com
Date: Tue Apr 9 12:35:33 2013 +0200
secur32: Added support for TLS 1.1 and TLS 1.2 on Mac.
diff --git a/dlls/secur32/schannel_macosx.c b/dlls/secur32/schannel_macosx.c
index 5ec06cf..27bb667 100644
--- a/dlls/secur32/schannel_macosx.c
+++ b/dlls/secur32/schannel_macosx.c
@@ -1007,7 +1007,25 @@ BOOL schan_imp_init(void)
supported_protocols = SP_PROT_SSL2_CLIENT | SP_PROT_SSL3_CLIENT |
SP_PROT_TLS1_0_CLIENT;
#if MAC_OS_X_VERSION_MAX_ALLOWED = 1080
-/* FIXME: Test max allowed version for TLS 1.1 and TLS 1.2 */
+if(SSLGetProtocolVersionMax) {
+SSLProtocol max_protocol;
+SSLContextRef ctx;
+OSStatus status;
+
+status = SSLNewContext(FALSE, ctx);
+if(status == noErr) {
+status = SSLGetProtocolVersionMax(ctx, max_protocol);
+if(status == noErr) {
+if(max_protocol = kTLSProtocol11)
+supported_protocols |= SP_PROT_TLS1_1_CLIENT;
+if(max_protocol = kTLSProtocol12)
+supported_protocols |= SP_PROT_TLS1_2_CLIENT;
+}
+SSLDisposeContext(ctx);
+}else {
+WARN(SSLNewContext failed\n);
+}
+}
#endif
return TRUE;
Re: Wine Gecko 2.20-beta1
On 03/07/13 12:07, Jacek Caban wrote:
Hi all,
I've uploaded a new Gecko builds to SourceForge [1]. This one is based on
Firefox 20 beta and, other than usual update of Gecko, it should fix some
crashes like bug b*ug 32753* http://bugs.winehq.org/show_bug.cgi?id=32753.
To test them, you need recent Wine Git version with the attached patch. As
usually, grab the build from SourceForge, put it in the right place [2] and
run patched Wine. All help with testing is appreciated!
Thanks,
Jacek
[1] http://sourceforge.net/projects/wine/files/Wine%20Gecko/2.20-beta1/
[2] http://wiki.winehq.org/Gecko
The release is pretty close, but it needs some more tests. If anyone is
willing to help, I've attached a rebased patch (nsiface.idl changed a
bit in Wine since I uploaded the last beta build).
Thanks,
Jacek
diff --git a/dlls/appwiz.cpl/addons.c b/dlls/appwiz.cpl/addons.c
index 2a47b84..cdaf7e2 100644
--- a/dlls/appwiz.cpl/addons.c
+++ b/dlls/appwiz.cpl/addons.c
@@ -53,7 +53,7 @@
WINE_DEFAULT_DEBUG_CHANNEL(appwizcpl);
-#define GECKO_VERSION 1.9
+#define GECKO_VERSION 2.20-beta1
#ifdef __i386__
#define ARCH_STRING x86
diff --git a/dlls/mshtml/htmlelem2.c b/dlls/mshtml/htmlelem2.c
index 24aaba5..516a3f5 100644
--- a/dlls/mshtml/htmlelem2.c
+++ b/dlls/mshtml/htmlelem2.c
@@ -1253,22 +1253,22 @@ static HRESULT WINAPI
HTMLElement2_getElementsByTagName(IHTMLElement2 *iface, BS
IHTMLElementCollection
**pelColl)
{
HTMLElement *This = impl_from_IHTMLElement2(iface);
-nsIDOMNodeList *nslist;
+nsIDOMHTMLCollection *nscol;
nsAString tag_str;
nsresult nsres;
TRACE((%p)-(%s %p)\n, This, debugstr_w(v), pelColl);
nsAString_InitDepend(tag_str, v);
-nsres = nsIDOMHTMLElement_GetElementsByTagName(This-nselem, tag_str,
nslist);
+nsres = nsIDOMHTMLElement_GetElementsByTagName(This-nselem, tag_str,
nscol);
nsAString_Finish(tag_str);
if(NS_FAILED(nsres)) {
ERR(GetElementByTagName failed: %08x\n, nsres);
return E_FAIL;
}
-*pelColl = create_collection_from_nodelist(This-node.doc, nslist);
-nsIDOMNodeList_Release(nslist);
+*pelColl = create_collection_from_htmlcol(This-node.doc, nscol);
+nsIDOMHTMLCollection_Release(nscol);
return S_OK;
}
diff --git a/dlls/mshtml/nsiface.idl b/dlls/mshtml/nsiface.idl
index 76f8ed0..a7fc344 100644
--- a/dlls/mshtml/nsiface.idl
+++ b/dlls/mshtml/nsiface.idl
@@ -23,7 +23,7 @@
* compatible with XPCOM, usable in C code.
*/
-cpp_quote(#define GECKO_VERSION \1.9\)
+cpp_quote(#define GECKO_VERSION \2.20-beta1\)
cpp_quote(#define GECKO_VERSION_STRING \Wine Gecko \ GECKO_VERSION)
import wtypes.idl;
@@ -179,10 +179,7 @@ typedef nsISupports nsIDOMHistory;
typedef nsISupports nsIDOMNavigator;
typedef nsISupports nsIDOMMediaQueryList;
typedef nsISupports nsIDOMScreen;
-typedef nsISupports nsIDOMCrypto;
-typedef nsISupports nsIDOMPkcs11;
typedef nsISupports nsIAnimationFrameListener;
-typedef nsISupports nsIDOMMozURLProperty;
typedef nsISupports nsIDOMStorageList;
typedef nsISupports nsILocalFile;
typedef nsISupports nsIDOMHTMLMenuElement;
@@ -195,6 +192,7 @@ typedef nsISupports nsIDOMBlob;
typedef nsISupports nsIPrivacyTransitionObserver;
typedef nsISupports nsIDOMHTMLPropertiesCollection;
typedef nsISupports mozIDOMApplication;
+typedef nsISupports nsILoadGroupConnectionInfo;
typedef void *JSContext;
typedef void *JSObject;
@@ -226,7 +224,7 @@ interface nsIFactory : nsISupports
[
object,
-uuid(59e7e77a-38e4-11d4-8cf5-0060b0fc14a3),
+uuid(6aef11c4-8615-44a6-9711-98f43805693d),
local
]
interface nsIMemory : nsISupports
@@ -236,6 +234,7 @@ interface nsIMemory : nsISupports
void Free(void *_ptr);
nsresult HeapMinimize(bool immediate);
nsresult IsLowMemory(bool *_retval);
+nsresult IsLowMemoryPlatform(bool *_retval);
}
[
@@ -521,7 +520,7 @@ interface nsIStreamListener : nsIRequestObserver
[
object,
-uuid(3de0a31c-feaf-400f-9f1e-4ef71f8b20cc),
+uuid(19501006-46e3-4634-b97d-26eff894b4d3),
local
]
interface nsILoadGroup : nsIRequest
@@ -536,11 +535,12 @@ interface nsILoadGroup : nsIRequest
nsresult GetActiveCount(uint32_t *aActiveCount);
nsresult GetNotificationCallbacks(nsIInterfaceRequestor
**aNotificationCallbacks);
nsresult SetNotificationCallbacks(nsIInterfaceRequestor
*aNotificationCallbacks);
+nsresult GetConnectionInfo(nsILoadGroupConnectionInfo **aConnectionInfo);
}
[
object,
-uuid(98f3b51b-bb55-4276-a43c-db636f8d77e3),
+uuid(2a8a7237-c1e2-4de7-b669-2002af29e42d),
local
]
interface nsIChannel : nsIRequest
@@ -557,8 +557,8 @@ interface nsIChannel : nsIRequest
nsresult SetContentType(const nsACString *aContentType);
nsresult GetContentCharset(nsACString *aContentCharset);
nsresult SetContentCharset(const nsACString *aContentCharset);
-nsresult GetContentLength(int32_t *aContentLength
Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.
Hi Juan, On 03/29/13 18:19, Juan Lang wrote: Hi Jacek, thanks for the detailed reply. On Fri, Mar 29, 2013 at 3:02 AM, Jacek Caban ja...@codeweavers.com mailto:ja...@codeweavers.com wrote: Each protocol has two kinds of enable/disable flags: enabled and disabled by default. Those have different default values for each protocol and there are registry setting allowing to change each of them. Only enabled protocols are used at all. This patch limits enabled protocols to those that we can really support. If an application asks schannel to use default set of protocols (which I'd expect them to do unless they have a good reason), schannel will use all enabled protocols that are not disabled by default. An alternative to default set of protocols is listing each allowed separately. This means that if protocol is enabled and disabled by default it won't be used unless application explicitly asks for it. SSL2 is such a protocol by default. Do you think we should do this differently? Yes, my suggestion here is to explicitly disable SSL2 support altogether. GnuTLS doesn't support it, and having behavior that differs between Linux and Mac can be kind of maddening. I can imagine something like, embedded browser doesn't work for game X, with lots of works for me reports and the occasional fails here too, only to discover that it works on Mac but not Linux. The additional cost of a difference in behavior doesn't seem worth it, especially when the protocol itself really should have died long ago. Most of the argument could be used against enabling TLS 1.1 and TLS 1.2, because it's not present on older Macs (nor enabled by default on Windows), so we'll have different behaviour. That's sadly something we have to live with. Anyway, I'm all for trying to avoid using SSL2, but I'd like to be a bit less extreme. How about this patch: http://source.winehq.org/patches/data/95298 With this patch, SSL2 will be completely disabled in default Wine config, but it will be still possible to enable by registries, if someone really needs it. Jacek
Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.
Hi Ken,
On 03/28/13 20:31, Ken Thomases wrote:
On Mar 28, 2013, at 6:05 AM, Jacek Caban wrote:
--- a/dlls/secur32/schannel_macosx.c
+++ b/dlls/secur32/schannel_macosx.c
@@ -630,6 +630,11 @@ static OSStatus schan_push_adapter(SSLConnectionRef
transport, const void *buff,
return ret;
}
+DWORD schan_imp_enabled_protocols(void)
+{
+/* NOTE: No support for TLS 1.1 and TLS 1.2 */
+return SP_PROT_SSL2_CLIENT | SP_PROT_SSL3_CLIENT |
SP_PROT_TLS1_0_CLIENT;
+}
Mac OS X 10.8 introduced support for TLS 1.1 and 1.2. You can test at build
time with:
#if MAC_OS_X_VERSION_MAX_ALLOWED = 1080
...
#else
...
#endif
If we want to support building on 10.8 for deployment to earlier versions,
we'd do something like:
#if MAC_OS_X_VERSION_MAX_ALLOWED = 1080
SSLProtocol maxProtocol;
if (SSLGetProtocolVersionMax != NULL
SSLGetProtocolVersionMax(context, maxProtocol) == noErr)
{
... compare maxProtocol against kTLSProtocol11 and
kTLSProtocol12 ...
}
...
#else
...
#endif
Thanks for the pointers, I've been meaning to explore it as follow-up.
My problem is that I'm still on 10.6 with Xcode 3.2. Would you mind
taking care of the patch?
The idea is that SSLGetProtocolVersionMax() would be weak linked, so we'd
check if it was actually available before calling it. Of course, the other
complication is that that function requires a context parameter, but we can
create one just for the query if we're interested in the framework
capabilities (as opposed to what's been configured for a particular context).
Yes, in this case we're only interested in framework capabilities. We
should determine protocols used for given context ourselves, based on
caller's requested protocol and confuration, and pass that to framework.
Setting up framework is not implemented yet, I have patches for that
that I want to test a bit more before sending.
Thanks,
Jacek
Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.
Hi Juan,
On 03/28/13 21:55, Juan Lang wrote:
On Thu, Mar 28, 2013 at 12:31 PM, Ken Thomases k...@codeweavers.com
mailto:k...@codeweavers.com wrote:
On Mar 28, 2013, at 6:05 AM, Jacek Caban wrote:
--- a/dlls/secur32/schannel_macosx.c
+++ b/dlls/secur32/schannel_macosx.c
@@ -630,6 +630,11 @@ static OSStatus
schan_push_adapter(SSLConnectionRef transport, const void *buff,
return ret;
}
+DWORD schan_imp_enabled_protocols(void)
+{
+/* NOTE: No support for TLS 1.1 and TLS 1.2 */
+return SP_PROT_SSL2_CLIENT | SP_PROT_SSL3_CLIENT |
SP_PROT_TLS1_0_CLIENT;
Do we really want to continue supporting SSL2? It's got a number of
vulnerabilities, and is disabled pretty much everywhere by now:
http://en.wikipedia.org/wiki/Transport_Layer_Security#SSL_2.0
I implemented it the way it's done in Windows. It's a bit
under-documented and contains usual MSDN mistakes, so let me explain
what I found when testing Windows (most of it is implemented by
http://source.winehq.org/git/wine.git/commitdiff/0f2e0365ea1f5c6baba4cfd9c0ff69defe66d7ea).
Each protocol has two kinds of enable/disable flags: enabled and
disabled by default. Those have different default values for each
protocol and there are registry setting allowing to change each of them.
Only enabled protocols are used at all. This patch limits enabled
protocols to those that we can really support. If an application asks
schannel to use default set of protocols (which I'd expect them to do
unless they have a good reason), schannel will use all enabled
protocols that are not disabled by default. An alternative to default
set of protocols is listing each allowed separately.
This means that if protocol is enabled and disabled by default it
won't be used unless application explicitly asks for it. SSL2 is such a
protocol by default. Do you think we should do this differently?
Thanks,
Jacek
Re: urlmon: Fix handling of mailto URIs in CoInternetCombineUrlEx.
Hi Hans,
On 03/27/13 10:43, Hans Leidekker wrote:
---
dlls/urlmon/tests/uri.c | 63
+++
dlls/urlmon/uri.c | 15 ---
2 files changed, 69 insertions(+), 9 deletions(-)
diff --git a/dlls/urlmon/tests/uri.c b/dlls/urlmon/tests/uri.c
index e5faa81..29bd859 100644
--- a/dlls/urlmon/tests/uri.c
+++ b/dlls/urlmon/tests/uri.c
@@ -7005,8 +7005,61 @@ static const uri_combine_test uri_combine_tests[] = {
{URL_SCHEME_FILE,S_OK},
{URLZONE_INVALID,E_NOTIMPL}
}
+},
+{ http://winehq.org,0,
+mailto://,0,
+0,S_OK,FALSE,
You are changing parts that are not combining-specific. Since the
relative URI is absolute in this case, there is not much happening other
than parsing the URI during combining. Those tests are fine, but please
add also tests for CreateUri, see uri_tests array. This should be
straightforward based on tests you already wrote.
diff --git a/dlls/urlmon/uri.c b/dlls/urlmon/uri.c
index 0f6ee31..c6b5166 100644
--- a/dlls/urlmon/uri.c
+++ b/dlls/urlmon/uri.c
@@ -422,7 +422,7 @@ static inline BOOL is_hierarchical_uri(const WCHAR **ptr,
const parse_data *data
else if(is_hierarchical_scheme(data-scheme_type) (*ptr)[0] == '\\'
(*ptr)[1] == '\\') {
*ptr += 2;
return TRUE;
-} else if(check_hierarchical(ptr))
+} else if(data-scheme_type != URL_SCHEME_MAILTO
check_hierarchical(ptr))
return TRUE;
*ptr = start;
@@ -2980,6 +2980,9 @@ static BOOL canonicalize_path_opaque(const parse_data
*data, Uri *uri, DWORD fla
const BOOL known_scheme = data-scheme_type != URL_SCHEME_UNKNOWN;
const BOOL is_file = data-scheme_type == URL_SCHEME_FILE;
const BOOL is_mk = data-scheme_type == URL_SCHEME_MK;
+const BOOL is_javascript = data-scheme_type == URL_SCHEME_JAVASCRIPT;
+const BOOL is_mailto = data-scheme_type == URL_SCHEME_MAILTO;
+DWORD offset = 0;
if(!data-path) {
uri-path_start = -1;
@@ -2996,7 +2999,7 @@ static BOOL canonicalize_path_opaque(const parse_data
*data, Uri *uri, DWORD fla
}
/* For javascript: URIs, simply copy path part without any
canonicalization */
-if(data-scheme_type == URL_SCHEME_JAVASCRIPT) {
+if(is_javascript) {
if(!computeOnly)
memcpy(uri-canon_uri+uri-canon_len, data-path,
data-path_len*sizeof(WCHAR));
uri-path_len = data-path_len;
@@ -3007,7 +3010,11 @@ static BOOL canonicalize_path_opaque(const parse_data
*data, Uri *uri, DWORD fla
/* Windows doesn't allow a // to appear after the scheme
* of a URI, if it's an opaque URI.
*/
-if(data-scheme *(data-path) == '/' *(data-path+1) == '/') {
+if (is_mailto *(data-path) == '/' *(data-path+1) == '/') {
+if (!*(data-path+2)) uri-path_start = -1;
+offset += 2;
I think this should be done in parsing phrase, not during canonicalization.
Jacek
Wine Gecko 2.20-beta1
Hi all,
I've uploaded a new Gecko builds to SourceForge [1]. This one is based on
Firefox 20 beta and, other than usual update of Gecko, it should fix some
crashes like bug b*ug 32753* http://bugs.winehq.org/show_bug.cgi?id=32753. To
test them, you need recent Wine Git version with the attached patch. As
usually, grab the build from SourceForge, put it in the right place [2] and run
patched Wine. All help with testing is appreciated!
Thanks,
Jacek
[1] http://sourceforge.net/projects/wine/files/Wine%20Gecko/2.20-beta1/
[2] http://wiki.winehq.org/Gecko
diff --git a/dlls/appwiz.cpl/addons.c b/dlls/appwiz.cpl/addons.c
index 2a47b84..cdaf7e2 100644
--- a/dlls/appwiz.cpl/addons.c
+++ b/dlls/appwiz.cpl/addons.c
@@ -53,7 +53,7 @@
WINE_DEFAULT_DEBUG_CHANNEL(appwizcpl);
-#define GECKO_VERSION 1.9
+#define GECKO_VERSION 2.20-beta1
#ifdef __i386__
#define ARCH_STRING x86
diff --git a/dlls/mshtml/htmlelem2.c b/dlls/mshtml/htmlelem2.c
index 24aaba5..516a3f5 100644
--- a/dlls/mshtml/htmlelem2.c
+++ b/dlls/mshtml/htmlelem2.c
@@ -1253,22 +1253,22 @@ static HRESULT WINAPI
HTMLElement2_getElementsByTagName(IHTMLElement2 *iface, BS
IHTMLElementCollection
**pelColl)
{
HTMLElement *This = impl_from_IHTMLElement2(iface);
-nsIDOMNodeList *nslist;
+nsIDOMHTMLCollection *nscol;
nsAString tag_str;
nsresult nsres;
TRACE((%p)-(%s %p)\n, This, debugstr_w(v), pelColl);
nsAString_InitDepend(tag_str, v);
-nsres = nsIDOMHTMLElement_GetElementsByTagName(This-nselem, tag_str,
nslist);
+nsres = nsIDOMHTMLElement_GetElementsByTagName(This-nselem, tag_str,
nscol);
nsAString_Finish(tag_str);
if(NS_FAILED(nsres)) {
ERR(GetElementByTagName failed: %08x\n, nsres);
return E_FAIL;
}
-*pelColl = create_collection_from_nodelist(This-node.doc, nslist);
-nsIDOMNodeList_Release(nslist);
+*pelColl = create_collection_from_htmlcol(This-node.doc, nscol);
+nsIDOMHTMLCollection_Release(nscol);
return S_OK;
}
diff --git a/dlls/mshtml/nsiface.idl b/dlls/mshtml/nsiface.idl
index 647566f..0bae46c 100644
--- a/dlls/mshtml/nsiface.idl
+++ b/dlls/mshtml/nsiface.idl
@@ -23,7 +23,7 @@
* compatible with XPCOM, usable in C code.
*/
-cpp_quote(#define GECKO_VERSION \1.9\)
+cpp_quote(#define GECKO_VERSION \2.20-beta1\)
cpp_quote(#define GECKO_VERSION_STRING \Wine Gecko \ GECKO_VERSION)
import wtypes.idl;
@@ -179,10 +179,7 @@ typedef nsISupports nsIDOMHistory;
typedef nsISupports nsIDOMNavigator;
typedef nsISupports nsIDOMMediaQueryList;
typedef nsISupports nsIDOMScreen;
-typedef nsISupports nsIDOMCrypto;
-typedef nsISupports nsIDOMPkcs11;
typedef nsISupports nsIAnimationFrameListener;
-typedef nsISupports nsIDOMMozURLProperty;
typedef nsISupports nsIDOMStorageList;
typedef nsISupports nsILocalFile;
typedef nsISupports nsIDOMHTMLMenuElement;
@@ -195,6 +192,7 @@ typedef nsISupports nsIDOMBlob;
typedef nsISupports nsIPrivacyTransitionObserver;
typedef nsISupports nsIDOMHTMLPropertiesCollection;
typedef nsISupports mozIDOMApplication;
+typedef nsISupports nsILoadGroupConnectionInfo;
typedef void *JSContext;
typedef void *JSObject;
@@ -226,7 +224,7 @@ interface nsIFactory : nsISupports
[
object,
-uuid(59e7e77a-38e4-11d4-8cf5-0060b0fc14a3),
+uuid(6aef11c4-8615-44a6-9711-98f43805693d),
local
]
interface nsIMemory : nsISupports
@@ -236,6 +234,7 @@ interface nsIMemory : nsISupports
void Free(void *_ptr);
nsresult HeapMinimize(bool immediate);
nsresult IsLowMemory(bool *_retval);
+nsresult IsLowMemoryPlatform(bool *_retval);
}
[
@@ -521,7 +520,7 @@ interface nsIStreamListener : nsIRequestObserver
[
object,
-uuid(3de0a31c-feaf-400f-9f1e-4ef71f8b20cc),
+uuid(19501006-46e3-4634-b97d-26eff894b4d3),
local
]
interface nsILoadGroup : nsIRequest
@@ -536,11 +535,12 @@ interface nsILoadGroup : nsIRequest
nsresult GetActiveCount(uint32_t *aActiveCount);
nsresult GetNotificationCallbacks(nsIInterfaceRequestor
**aNotificationCallbacks);
nsresult SetNotificationCallbacks(nsIInterfaceRequestor
*aNotificationCallbacks);
+nsresult GetConnectionInfo(nsILoadGroupConnectionInfo **aConnectionInfo);
}
[
object,
-uuid(98f3b51b-bb55-4276-a43c-db636f8d77e3),
+uuid(2a8a7237-c1e2-4de7-b669-2002af29e42d),
local
]
interface nsIChannel : nsIRequest
@@ -557,8 +557,8 @@ interface nsIChannel : nsIRequest
nsresult SetContentType(const nsACString *aContentType);
nsresult GetContentCharset(nsACString *aContentCharset);
nsresult SetContentCharset(const nsACString *aContentCharset);
-nsresult GetContentLength(int32_t *aContentLength);
-nsresult SetContentLength(int32_t aContentLength);
+nsresult GetContentLength(int64_t *aContentLength);
+nsresult SetContentLength(int64_t aContentLength);
nsresult Open(nsIInputStream **_retval);
nsresult
Re: [1/3] wininet: Remember that headers were not received.
Hi Ričardas, On 02/27/13 00:07, Ričardas Barkauskas wrote: @@ -5708,6 +5708,7 @@ static INT HTTP_GetResponseHeaders(http_request_t *request, BOOL clear) heap_free(request-rawHeaders); request-rawHeaders = heap_strdupW(szDefaultHeader); +request-flags |= INTERNET_REQFLAG_NO_HEADERS; I don't think we want to maintain valid request-flags for that particular flag. Checking for request-rawHeaders should be enough. If we avoided setting rawHeaders here, we'd achieve the same behaviour. Jacek
Re: IE 6 related urlmon:protocol failures
On 02/18/13 12:49, Francois Gouget wrote: On Sat, 16 Feb 2013, Jacek Caban wrote: On 2/16/13 1:28 AM, Francois Gouget wrote: However I think the test should still not fail. So I'm looking for a volunteer to either get the test to succeed with urlmon 6.0, or find a way to detect it and skip these three tests. I believe it's time to skip those tests on IE6 then. I will send a patch. Thanks for the analyze. wininet:http has the exact same problem. http://wiki.winehq.org/WineTestBotVMs#wininet It seems to me like those tests could be simply removed. The part that is failing is just a preparation for tests reading from cache. And the important part of those tests always fail on Windows (the comment says that they fail on new Windows, but AFAICS on both testbots, they always fail). Those tests are also marked todo_wine. It would be great to make proper use of cache in our http implementation, but that will require writing more tests anyway. /me trying to push my luckg. Don't hesitate to push more if needed :) Cheers, Jacek
Re: [PATCH 1/2] wininet: Address string should never be converted to UNICODE in status notifications
On 02/18/13 15:50, Marvin wrote: 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 http://testbot.winehq.org/JobDetails.pl?Key=24441 Your paranoid android. That's because TestBot still can't update to current Wine version: http://testbot.winehq.org/JobDetails.pl?Key=24423 Jacek
Re: IE 6 related urlmon:protocol failures
On 2/16/13 1:28 AM, Francois Gouget wrote: However I think the test should still not fail. So I'm looking for a volunteer to either get the test to succeed with urlmon 6.0, or find a way to detect it and skip these three tests. I believe it's time to skip those tests on IE6 then. I will send a patch. Thanks for the analyze. Cheers, Jacek
Re: [PATCH 2/2] wininet: Set available bytes in InternetQueryDataAvailable even if it ends up in async call
On 02/14/13 18:13, Marvin wrote: 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 http://testbot.winehq.org/JobDetails.pl?Key=24401 Your paranoid android. === WINEBUILD (build) === Patch failed to apply That's because TestBot failed to update Wine tree: http://testbot.winehq.org/JobDetails.pl?Key=24387log_101=1#k101 Here is a successful run: http://testbot.winehq.org/JobDetails.pl?Key=24400 Cheers, Jacek
Re: [PATCH 2/4] urlmon: Fixed QueryInfo tests during BINDSTATUS_PROXYDETECTING notification
On 02/12/13 17:32, Marvin wrote: 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 http://testbot.winehq.org/JobDetails.pl?Key=24361 Your paranoid android. === WVISTAADM (32 bit url) === url.c:1868: Test failed: binding failed: 800c0005, expected url.c:1879: Test failed: res = 2ee7, expected url.c:3035: Test failed: IMoniker_BindToStorage failed: 800c0005 url.c:3036: Test failed: unk == NULL (...) These seem to be (random) network failure. I've got it all succeeding before sending patches: https://testbot.winehq.org/JobDetails.pl?Key=24358 Jacek
Re: Makedll.rules.in: Explicitly specify DLL name when building importlib
On 01/30/13 20:09, Alexandre Julliard wrote: Jacek Caban ja...@codeweavers.com writes: --- dlls/Makedll.rules.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Why do you need this? My use case is not really valid as is for Wine. I was experimenting with running parts of Wine on Windows, but I needed to change some DLL names. With this patch it's as easy as changing MODULE in Makefile.in files and copying .spec file. This way I get renamed DLLs built correctly and other DLLs depending on the new name without touching anything else. The above is not really a reason to put it in Wine. However, I thought that this behaviour is logical consequence of having MODULE and IMPORTLIB separately in Makefiles (as opposed to, say, IMPORTLIB=1 and using MODULE for its name), so I sent the patch. Jacek
Re: [PATCH 1/8] winhttp: Use schannel in netconn_secure_connect if OpenSSL is not available
On 01/22/13 20:21, Alexandre Julliard wrote: Jacek Caban ja...@codeweavers.com writes: The following patch makes winhttp use schannel instead of OpenSSL for SSL (HTTPS) connections. It is made to be as minimal change as possible (but complete enough to replace all OpenSSL functionality) to limit the scope for potential regressions. Further changes and improvements are left for later. It doesn't work here: I think I found the problem. I will fix a new version. Thanks, Jacek
Re: [PATCH 6/8] winhttp: Added schannel-based netconn_get_cipher_strength implementation
On 01/22/13 17:22, Hans Leidekker wrote: On Tue, 2013-01-22 at 16:49 +0100, Jacek Caban wrote: @@ -1460,6 +1460,13 @@ int netconn_get_cipher_strength( netconn_t *conn ) pSSL_CIPHER_get_bits( cipher, bits ); return bits; #else -return 0; +SecPkgContext_ConnectionInfo conn_info; +SECURITY_STATUS res; + +if (!conn-secure) return 0; +res = QueryContextAttributesW(conn-ssl_ctx, SECPKG_ATTR_REMOTE_CERT_CONTEXT, (void*)conn_info); That should be SECPKG_ATTR_CONNECTION_INFO. Good catch, thanks. I will send a fixed version. Jacek
Re: [PATCH 2/3] vbscript: Added VBScript.RegExp version 5.5 typelib
Hi Piotr, On 01/18/13 11:48, Piotr Caban wrote: +#define DISPID_SUBMATCHES_ITEM 0 +#define DISPID_SUBMATCHES_COUNT 1 +#define DISPID_SUBMATCHES__NEWENUM -4 + +#define DISPID_MATCHCOLLECTION_ITEM 0 +#define DISPID_MATCHCOLLECTION_COUNT1 +#define DISPID_MATCHCOLLECTION__NEWENUM -4 + +#define DISPID_MATCH_VALUE 0 Please use DISPID_VALUE and DISPID_NEWENUM instead. Jacek
Re: [PATCH (try2)] ieframe: Strip 'file://' from file URLs in BEFORENAVIGATE2 callbacks
Hi Andrew,
On 01/09/13 16:56, Andrew Eikum wrote:
---
This should fix the Office 2007 half of Bug 17271.
try2: Fix test failures on XP by expanding the expected path name.
dlls/ieframe/navigate.c | 7 +-
dlls/ieframe/tests/webbrowser.c | 56
++---
2 files changed, 59 insertions(+), 4 deletions(-)
0001-ieframe-Strip-file-from-file-URLs-in-BEFORENAVIGATE2.patch
diff --git a/dlls/ieframe/navigate.c b/dlls/ieframe/navigate.c
index dd4a976..2601ba8 100644
--- a/dlls/ieframe/navigate.c
+++ b/dlls/ieframe/navigate.c
@@ -566,6 +566,8 @@ static void on_before_navigate2(DocHost *This, LPCWSTR
url, SAFEARRAY *post_data
VARIANT var_url, var_flags, var_frame_name, var_post_data,
var_post_data2, var_headers;
DISPPARAMS dispparams;
VARIANTARG params[7];
+WCHAR file_path[MAX_PATH];
+DWORD file_path_len = sizeof(file_path) / sizeof(*file_path);
dispparams.cArgs = 7;
dispparams.cNamedArgs = 0;
@@ -607,7 +609,10 @@ static void on_before_navigate2(DocHost *This, LPCWSTR
url, SAFEARRAY *post_data
V_VT(params+5) = (VT_BYREF|VT_VARIANT);
V_VARIANTREF(params+5) = var_url;
V_VT(var_url) = VT_BSTR;
-V_BSTR(var_url) = SysAllocString(url);
+if(PathCreateFromUrlW(url, file_path, file_path_len, 0) == S_OK)
+V_BSTR(var_url) = SysAllocString(file_path);
+else
+V_BSTR(var_url) = SysAllocString(url);
V_VT(params+6) = (VT_DISPATCH);
V_DISPATCH(params+6) = (IDispatch*)This-wb;
diff --git a/dlls/ieframe/tests/webbrowser.c b/dlls/ieframe/tests/webbrowser.c
index 25cce8a..70c785b 100644
--- a/dlls/ieframe/tests/webbrowser.c
+++ b/dlls/ieframe/tests/webbrowser.c
@@ -151,7 +151,7 @@ static VARIANT_BOOL exvb;
static IWebBrowser2 *wb;
static HWND container_hwnd, shell_embedding_hwnd;
-static BOOL is_downloading, is_first_load, use_container_olecmd, test_close,
is_http, use_container_dochostui;
+static BOOL is_downloading, is_first_load, use_container_olecmd, test_close,
is_http, is_file, use_container_dochostui;
static HRESULT hr_dochost_TranslateAccelerator = E_NOTIMPL;
static HRESULT hr_site_TranslateAccelerator = E_NOTIMPL;
static const char *current_url;
@@ -690,8 +690,14 @@ static void test_OnBeforeNavigate(const VARIANT *disp,
const VARIANT *url, const
ok(V_VT(V_VARIANTREF(url)) == VT_BSTR, V_VT(V_VARIANTREF(url))=%d,
expected VT_BSTR\n,
V_VT(V_VARIANTREF(url)));
ok(V_BSTR(V_VARIANTREF(url)) != NULL, V_BSTR(V_VARIANTREF(url)) ==
NULL\n);
-ok(!strcmp_wa(V_BSTR(V_VARIANTREF(url)), current_url), unexpected
url %s, expected %s\n,
- wine_dbgstr_w(V_BSTR(V_VARIANTREF(url))), current_url);
+if(is_file){
+char full_path[MAX_PATH];
+GetLongPathNameA(current_url, full_path, sizeof(full_path));
I think expanding the path would be better done while constructing the URL
instead of here.
+test_DoVerb(webbrowser);
+test_Navigate2(webbrowser, file_url);
+
+IWebBrowser2_Release(webbrowser);
This is not enough for proper clean up. You may verify that by checking ref
count here. At least setting client site to NULL is needed.
Cheers,
Jacek
Re: [PATCH 1/7] d3dx9: Handle invalid byte code in D3DXFindShaderComment().
On 01/02/13 14:38, Matteo Bruni wrote:
2013/1/1 Rico Schüller kgbric...@web.de:
---
dlls/d3dx9_36/shader.c | 16 +++-
dlls/d3dx9_36/tests/shader.c | 21 +
2 Dateien geändert, 32 Zeilen hinzugefügt(+), 5 Zeilen entfernt(-)
+static inline BOOL is_valid_bytecode(DWORD token)
+{
+token = 0x;
+return token == 0x || token == 0xFFFE;
+}
This series looks good to me, but I'd prefer you use lowercase
hexadecimal constants in the future.
While you are at this, it may be simplified to:
return token 0xfffe == 0xfffe;
Cheers,
Jacek
Re: msxml3: Add custom handling for DISPID_VALUE for node map
Hi Nikolay, On 12/20/12 11:29, Nikolay Sivov wrote: On 12/19/2012 17:22, Jacek Caban wrote: On 12/19/12 08:24, Nikolay Sivov wrote: Add custom handling for DISPID_VALUE for node map If this has to be done for all IDispatchEx objects with DISPID_VALUE, it may be worth considering changing dispex.c. Something like: if(id == DISPID_VALUE wFlags == DISPATCH_METHOD) wFlags |= DISPATCH_PROPERTYGET; before ITypeInfo::Invoke call should do the trick and replace both this patch and already committed node list patch. So I did more testing and it doesn't work like that for all DISPID_VALUE methods, what a surprise: http://www.winehq.org/pipermail/wine-patches/2012-December/120928.html Do you think it's still better to centrally change it in dispex.c and wrongly accept DISPATCH_METHOD when we shouldn't? I'm not really convinced by those tests, here is why: + +hr = IXMLDOMParseError_QueryInterface(error, IID_IDispatchEx, (void**)dispex); +ok(hr == S_OK, got 0x%08x\n, hr); + +V_VT(arg) = VT_I4; +V_I4(arg) = 0; +dispparams.cArgs = 1; +dispparams.cNamedArgs = 0; +dispparams.rgdispidNamedArgs = NULL; +dispparams.rgvarg = arg; + +V_VT(ret) = VT_EMPTY; +V_DISPATCH(ret) = (void*)0x1; +hr = IDispatchEx_Invoke(dispex, DISPID_VALUE, IID_NULL, 0, DISPATCH_METHOD, dispparams, ret, NULL, NULL); +ok(hr == DISP_E_MEMBERNOTFOUND, got 0x%08x\n, hr); +ok(V_VT(ret) == VT_EMPTY, got %d\n, V_VT(ret)); +ok(V_DISPATCH(ret) == (void*)0x1, got %p\n, V_DISPATCH(ret)); IXMLDOMParseError's DISPID_VALUE is errorCode property, which doesn't take arguments (in C world, it's no argument except retval). You're passing one argument, so the call should return error (I'd expect DISP_E_BADPARAMCOUNT, but well...). +hr = IXMLDOMSchemaCollection_QueryInterface(cache, IID_IDispatchEx, (void**)dispex); +ok(hr == S_OK, got 0x%08x\n, hr); + +V_VT(arg) = VT_I4; +V_I4(arg) = 0; +dispparams.cArgs = 1; +dispparams.cNamedArgs = 0; +dispparams.rgdispidNamedArgs = NULL; +dispparams.rgvarg = arg; + +V_VT(ret) = VT_EMPTY; +V_DISPATCH(ret) = (void*)0x1; +hr = IDispatchEx_Invoke(dispex, DISPID_VALUE, IID_NULL, 0, DISPATCH_METHOD, dispparams, ret, NULL, NULL); +ok(hr == DISP_E_MEMBERNOTFOUND, got 0x%08x\n, hr); +ok(V_VT(ret) == VT_EMPTY, got %d\n, V_VT(ret)); +ok(V_DISPATCH(ret) == (void*)0x1, got %p\n, V_DISPATCH(ret)); This one is indeed showing different behaviour, although the test uses an empty collection, so the call probably shouldn't work anyway. I'm not saying that there should be no custom implementations, but AFAICS majority of objects can be fixed by simple IDispatchEx implementation modification and if there are cases that still need fixing, then we can fall back to custom implementation. Cheers, Jacek
Re: msxml3: Add custom handling for DISPID_VALUE for node map
On 12/19/12 08:24, Nikolay Sivov wrote: Add custom handling for DISPID_VALUE for node map If this has to be done for all IDispatchEx objects with DISPID_VALUE, it may be worth considering changing dispex.c. Something like: if(id == DISPID_VALUE wFlags == DISPATCH_METHOD) wFlags |= DISPATCH_PROPERTYGET; before ITypeInfo::Invoke call should do the trick and replace both this patch and already committed node list patch. Cheers, Jacek
Re: wininet: Don't perform revocation checks when verifying a certificate.
Hi Hans, On 12/11/12 09:45, Hans Leidekker wrote: https://testbot.winehq.org/JobDetails.pl?Key=23300 is a test which shows that revocation checks fail for the certificate on outlook.com when passed straight to CertVerifyRevocation. The reason is that a CRL link specified in the certificate does not resolve. https://testbot.winehq.org/JobDetails.pl?Key=23301 is a test which makes a secure connection to outlook.com from wininet and shows that this succeeds. My conclusion is that native wininet doesn't perform revocation checks. Your tests prove that we should relax our verification on CERT_TRUST_IS_OFFLINE_REVOCATION or something similar. To prove that revocation checks are not made, a test with truly revoked cert would be needed. Jacek
Re: wshom: Basic support for REG_SZ values in RegRead() (try2)
Hi Nikolay, On 12/04/12 10:33, Nikolay Sivov wrote: +if (name[len] != '\\') +return HRESULT_FROM_WIN32(ERROR_PATH_NOT_FOUND); +/* skip '\' after root name */ +len++; + +/* at this point we have at least one '\' in path */ +val = strrchrW(name, '\\'); +keyname = SysAllocStringLen(name[len], val-name[len]); It's better, but now if there is only one backslash, you will have val-name[len] == -1. Thanks, Jacek
Re: mshtml: Added support for converting (some) argument types in builtin function calls using script engine
Hi Marvin, On 12/04/12 15:14, Marvin wrote: 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, Yeah, I can see that... so I might be wrong, but could you please double-check? Sure, there you go: https://testbot.winehq.org/JobDetails.pl?Key=23215 Jacek
Re: wshom: Basic support for REG_SZ values in RegRead()
Hi Nikolay,
On 12/03/12 12:29, Nikolay Sivov wrote:
+/* skip '\' after root name */
+if (name[len] == '\\') len++;
It looks like if name[len] != '\\', then name should be considered invalid.
+
+val = strlenW(name);
+while (name[--val] != '\\' val)
+;
+keyname = SysAllocStringLen(name[len], val-len);
This won't work if name has no backslashes.
+/* skip leading '\' */
+val++;
+
+type = REG_NONE;
+datalen = 0;
+ret = RegGetValueW(root, keyname, name[val], RRF_RT_ANY, type, NULL,
datalen);
+if (ret == ERROR_SUCCESS)
+{
+if (type == REG_SZ)
+{
+V_VT(value) = VT_BSTR;
+V_BSTR(value) = SysAllocStringLen(NULL, datalen-1);
+ret = RegGetValueW(root, keyname, name[val], RRF_RT_ANY, type,
V_BSTR(value), datalen);
+}
+else
+FIXME(type %d not handled\n, type);
This should return an error.
Jacek
Re: msvcp60: Remove superfluous semicolons
On 11/26/12 22:35, Andrew Talbot wrote: Jacek Caban wrote: It's probably better to change the macro to require the semicolon. Jacek The reason I did it that way was because there are two variants of the DEFINE_CXX_DATA macro, surrounded by an #ifndef construct: one comprising three struct declarations, all ending in semicolons, the other comprised three struct declarations (ending in semicolons) and one function definition (ending in a curly brace). I suppose I could move the function above the structs, but that would require forward declarations. Ah, sorry, missed the function there. Both solution seem equally fine then. Jacek
Wine Gecko versioning
Hi all, Since Wine Gecko 1.9 branching, the tip should bump to 1.10, but I think it's time to clean up the situation a bit. Such things usually don't bring much attention, but it should be decided in public. Right now we have three versioning schemes involved in Wine Gecko: - Wine version that applies to Gecko - Gecko version (since Firefox 5 and Mozilla's rapid releases it's the same as Firefox version) - Wine Gecko version, which is just a growing number, not really connected with any of above. These numbers currently have no meaning other that being different for different Wine Gecko releases. The idea is that Wine Gecko version could be just something based on other versions, that are more informative. It's not really possible to use Wine version, because the first version of Wine that will use new Gecko is not ultimately when Wine Gecko branches. Also multiple Wine versions use the same Wine Gecko. That leaves us with Gecko (Firefox version). We don't release on every Firefox release (every 6 weeks), so if we just used Firefox version, that would look strange (like Wine Gecko 18 followed by Wine Gecko 20). That can be mitigated by using it as a minor version. So the next few release would look like: - 1.9 (that's already in beta and will be the last release using old scheme) - 2.20 (assuming the next update will be 3 months from 1.9, which means Firefox 20) - 2.22 (assuming another 3 moths for the update). Any suggestions/comments welcomed. Cheers, Jacek
Re: Wine Gecko versioning
On 11/27/12 14:53, André Hentschel wrote: Am 27.11.2012 13:32, schrieb Jacek Caban: The idea is that Wine Gecko version could be just something based on other versions, that are more informative. It's not really possible to use Wine version, because the first version of Wine that will use new Gecko is not ultimately when Wine Gecko branches. Also multiple Wine versions use the same Wine Gecko. That leaves us with Gecko (Firefox version). We don't release on every Firefox release (every 6 weeks), so if we just used Firefox version, that would look strange (like Wine Gecko 18 followed by Wine Gecko 20). That can be mitigated by using it as a minor version. So the next few release would look like: - 1.9 (that's already in beta and will be the last release using old scheme) - 2.20 (assuming the next update will be 3 months from 1.9, which means Firefox 20) - 2.22 (assuming another 3 moths for the update). I like the idea, but why move to 2.x? 1.20, 1.22 and so on would perfectly fit. And if the gap between 1.9 and 1.20 is confusing someone, then he really has issues. I'm also fine with 2.x, i just think a new version scheme shouldn't necessarily lead to a version increment. You're right, 1.20 would fit, but I also don't see anything wrong with version increment. That said, I don't have strong preference here. Thanks, Jacek
Re: Wine Gecko versioning
On 11/27/12 16:33, Nikolay Sivov wrote: On 11/27/2012 17:26, Rosanne DiMesio wrote: On Tue, 27 Nov 2012 13:32:02 +0100 Jacek Caban ja...@codeweavers.com wrote: The idea is that Wine Gecko version could be just something based on other versions, that are more informative. Users don't care what version of Firefox the latest gecko is based on; most don't even realize it is based on Firefox. What does often confuse users, and which your suggested scheme doesn't address, is keeping track of which version of Wine uses which version of wine-gecko. As Nikolay said, we have a Wiki page for that. It's not something we can improve by different Gecko versioning. It's described here http://wiki.winehq.org/Gecko. I guess failure message loading gecko (in load_gecko()) could be improved adding GECKO_VERSION to it so user clearly see what version he needs to have if it's somehow not downloaded already. Sounds like a good idea, although we'll probably want to fix it by including version information in Gecko downloader. Thanks, Jacek
Re: msvcp60: Remove superfluous semicolons
On 11/26/12 8:42 PM, Andrew Talbot wrote: Changelog: msvcp60: Remove superfluous semicolons. diff --git a/dlls/msvcp60/exception.c b/dlls/msvcp60/exception.c index 761075e..b8c6921 100644 --- a/dlls/msvcp60/exception.c +++ b/dlls/msvcp60/exception.c @@ -137,7 +137,7 @@ void * __thiscall MSVCP_exception_vector_dtor(exception *this, unsigned int flag } DEFINE_RTTI_DATA0(exception, 0, .?AVexception@std@@); -DEFINE_CXX_DATA0(exception, MSVCP_exception_dtor); +DEFINE_CXX_DATA0(exception, MSVCP_exception_dtor) It's probably better to change the macro to require the semicolon. Jacek
Wine Gecko 1.9-beta1
Hi all,
Firefox 17 was released lately, meaning that Firefox 18 goes to beta
channel and it's the next target for out Gecko package update. I've
uploaded new Gecko builds to SourceForge [1]. To test them, you need
recent Wine Git version with the attached patch. As usually, grab the
build from SourceForge, put it in the right place [2] and run patched
Wine. All help with testing is appreciated!
Thanks,
Jacek
[1] http://sourceforge.net/projects/wine/files/Wine%20Gecko/1.9-beta1/
[2] http://wiki.winehq.org/Gecko
diff --git a/dlls/appwiz.cpl/addons.c b/dlls/appwiz.cpl/addons.c
index 60d2472..74ae43a 100644
--- a/dlls/appwiz.cpl/addons.c
+++ b/dlls/appwiz.cpl/addons.c
@@ -53,7 +53,7 @@
WINE_DEFAULT_DEBUG_CHANNEL(appwizcpl);
-#define GECKO_VERSION 1.8
+#define GECKO_VERSION 1.9-beta1
#ifdef __i386__
#define ARCH_STRING x86
diff --git a/dlls/mshtml/htmlnode.c b/dlls/mshtml/htmlnode.c
index b1f1b80..cf22838 100644
--- a/dlls/mshtml/htmlnode.c
+++ b/dlls/mshtml/htmlnode.c
@@ -1143,7 +1143,7 @@ static nsresult NSAPI HTMLDOMNode_traverse(void *ccp,
void *p, nsCycleCollection
TRACE(%p\n, This);
-describe_cc_node(This-ccref, sizeof(*This), HTMLDOMNode, cb);
+describe_cc_node(This-ccref, HTMLDOMNode, cb);
if(This-nsnode)
note_cc_edge((nsISupports*)This-nsnode, This-nsnode, cb);
diff --git a/dlls/mshtml/mshtml_private.h b/dlls/mshtml/mshtml_private.h
index d2c7b28..678ef3e 100644
--- a/dlls/mshtml/mshtml_private.h
+++ b/dlls/mshtml/mshtml_private.h
@@ -254,7 +254,7 @@ nsrefcnt (__cdecl
*ccref_decr)(nsCycleCollectingAutoRefCnt*,nsISupports*);
void (__cdecl *ccref_init)(nsCycleCollectingAutoRefCnt*,nsrefcnt);
void (__cdecl *ccref_unmark_if_purple)(nsCycleCollectingAutoRefCnt*);
void (__cdecl *ccp_init)(nsXPCOMCycleCollectionParticipant*,const
CCObjCallback*);
-void (__cdecl *describe_cc_node)(nsCycleCollectingAutoRefCnt*,size_t,const
char*,nsCycleCollectionTraversalCallback*);
+void (__cdecl *describe_cc_node)(nsCycleCollectingAutoRefCnt*,const
char*,nsCycleCollectionTraversalCallback*);
void (__cdecl *note_cc_edge)(nsISupports*,const
char*,nsCycleCollectionTraversalCallback*);
void init_dispex(DispatchEx*,IUnknown*,dispex_static_data_t*) DECLSPEC_HIDDEN;
diff --git a/dlls/mshtml/mutation.c b/dlls/mshtml/mutation.c
index 4b07c3f..87497f6 100644
--- a/dlls/mshtml/mutation.c
+++ b/dlls/mshtml/mutation.c
@@ -533,6 +533,11 @@ static void NSAPI
nsDocumentObserver_AttributeChanged(nsIDocumentObserver *iface
{
}
+static void NSAPI
nsDocumentObserver_AttributeSetToCurrentValue(nsIDocumentObserver *iface,
nsIDocument *aDocument,
+void *aElement, PRInt32 aNameSpaceID, nsIAtom *aAttribute)
+{
+}
+
static void NSAPI nsDocumentObserver_ContentAppended(nsIDocumentObserver
*iface, nsIDocument *aDocument,
nsIContent *aContainer, nsIContent *aFirstNewContent, PRInt32
aNewIndexInContainer)
{
@@ -715,6 +720,7 @@ static const nsIDocumentObserverVtbl nsDocumentObserverVtbl
= {
nsDocumentObserver_CharacterDataChanged,
nsDocumentObserver_AttributeWillChange,
nsDocumentObserver_AttributeChanged,
+nsDocumentObserver_AttributeSetToCurrentValue,
nsDocumentObserver_ContentAppended,
nsDocumentObserver_ContentInserted,
nsDocumentObserver_ContentRemoved,
diff --git a/dlls/mshtml/navigate.c b/dlls/mshtml/navigate.c
index 346524d..02826f2 100644
--- a/dlls/mshtml/navigate.c
+++ b/dlls/mshtml/navigate.c
@@ -138,7 +138,7 @@ static nsresult NSAPI nsInputStream_Close(nsIInputStream
*iface)
return NS_ERROR_NOT_IMPLEMENTED;
}
-static nsresult NSAPI nsInputStream_Available(nsIInputStream *iface, PRUint32
*_retval)
+static nsresult NSAPI nsInputStream_Available(nsIInputStream *iface, PRUint64
*_retval)
{
nsProtocolStream *This = impl_from_nsIInputStream(iface);
FIXME((%p)-(%p)\n, This, _retval);
@@ -1000,7 +1000,8 @@ HRESULT bind_mon_to_wstr(HTMLInnerWindow *window,
IMoniker *mon, WCHAR **ret)
static HRESULT read_post_data_stream(nsChannelBSC *This, nsChannel *nschannel)
{
-PRUint32 data_len = 0, available = 0;
+PRUint64 available = 0;
+PRUint32 data_len = 0;
char *data, *post_data;
nsresult nsres;
HRESULT hres = S_OK;
diff --git a/dlls/mshtml/nsiface.idl b/dlls/mshtml/nsiface.idl
index 744f45b..b59022b 100644
--- a/dlls/mshtml/nsiface.idl
+++ b/dlls/mshtml/nsiface.idl
@@ -23,7 +23,7 @@
* compatible with XPCOM, usable in C code.
*/
-cpp_quote(#define GECKO_VERSION \1.8\)
+cpp_quote(#define GECKO_VERSION \1.9-beta1\)
cpp_quote(#define GECKO_VERSION_STRING \Wine Gecko \ GECKO_VERSION)
import wtypes.idl;
@@ -67,6 +67,7 @@ typedef ULONGLONG PRUint64;
typedef uint64_t DOMTimeStamp;
typedef uint32_t nsLoadFlags;
+typedef int64_t PRTime;
/* Similar trick to stdint.h types for C++ bool type */
typedef unsigned char cpp_bool;
@@ -373,13 +374,13 @@ interface nsIWritableVariant : nsIVariant
[
object,
-uuid(fa9c7f6c-61b3-11d4-9877-00c04fa0cf4a),
+
Re: [PATCH 2/4] [cmd] Prevent external env vars causing tests to fail
On 11/20/12 12:39 AM, Ann and Jason Edmeades wrote: (I'll make this change in try 2, as I need to fix the broken NT4 stuff from patch 1 anyway) I think it's time to consider skipping all cmd tests on NT4. Adding @or_broken@ for NT4 mostly pollutes those tests. Jacek
Re: [PATCH 2/4] [cmd] Prevent external env vars causing tests to fail
On 11/20/12 16:00, Ann and Jason Edmeades wrote: On 20 November 2012 12:15, Jacek Caban ja...@codeweavers.com mailto:ja...@codeweavers.com wrote: On 11/20/12 12:39 AM, Ann and Jason Edmeades wrote: (I'll make this change in try 2, as I need to fix the broken NT4 stuff from patch 1 anyway) I think it's time to consider skipping all cmd tests on NT4. Adding @or_broken@ for NT4 mostly pollutes those tests. Jacek Please...please... please please I'm fed up with working around broken and missing functionality in NT4 for the tests - its taken longer sometimes than writing the code! Yeah, that's why I hope at some point we will obsolete NT4 just like we did with Win9x, but that's a subject for separated discussion. Let's concentrate on skipping individual tests for now. Who makes this decision, and if agreed, It's the usual, someone has to write a patch, send it and see if anyone objects and is accepted by Alexandre. In this case I don't expect anyone to object. how is the skipping done? See cmd_available in batch.c. The easiest way to skip NT4 would be probably to change it to return FALSE on this platform. Note that you can't explicitly test windows version, but if something like running a script that you know that fails only on NT4 would do the trick. Jacek
Re: [PATCH 7/9] ieframe: Forward more DocHost::Exec calls to embedders
On 11/12/12 13:21, Marvin wrote: 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 http://testbot.winehq.org/JobDetails.pl?Key=22897 Your paranoid android. === WNT4WSSP6 (32 bit webbrowser) === webbrowser.c:3289: Test failed: Creating WebBrowser object failed: 80040154 webbrowser.c:3289: Test failed: Creating WebBrowser object failed: 80040154 webbrowser.c:3289: Test failed: Creating WebBrowser object failed: 80040154 webbrowser.c:3428: Test failed: Creating WebBrowser object failed: 80040154 webbrowser: unhandled exception c005 at 0040810F === W2KPROSP4 (32 bit webbrowser) === webbrowser.c:2596: Test failed: expected GetOverridesKeyPath webbrowser.c:2601: Test failed: expected Invoke_SETSECURELOCKICON webbrowser.c:2602: Test failed: expected Invoke_FILEDOWNLOAD webbrowser.c:3015: Test failed: doc_disp == NULL webbrowser: unhandled exception c005 at 00402F21 That's TestBot not skipping on platforms that should be skipped. Jacek
Re: vbscript/tests: Skip some of the weekday tests if the first day of the week is not Sunday.
Hi Huw, On 10/31/12 12:04, Huw Davies wrote: --- dlls/vbscript/tests/api.vbs | 17 + 1 files changed, 9 insertions(+), 8 deletions(-) Did you see those failures on Windows? test.winehq.org shows only some Linux failures, which suggests that the problem is in implementation, not tests. Jacek
Re: [PATCH 4/5] include: Don't add d3d8 LP* interface typedefs for __WINESRC__.
On 10/30/12 12:01, Alexandre Julliard wrote: Henri Verbeet hverb...@gmail.com writes: We can certainly generate some more d3d headers from idl, and we probably should anyway, but I'm not sure how that's going to make the mechanism any more generic. I suppose we could do something like DECL_WINELIB_TYPE(struct IDirect3D8, *LPDIRECT3D8);, if that's much better. That would be better for headers written by hand, yes. But I'm not sure we want to add that at this point, since the goal should be to generate all interfaces from idl. It's unrelated to original problem here, but please keep in mind that other work on widl should also be done before switching to IDL files. Currently IDL-generated files are not really compatible with MSVC non-IDL-generated COM headers because widl adds some stuff like default includes. We probably need some way (command like arg or some tricky argument in IDL file) to instruct widl to generate pure headers. This may not be a big deal for Wine (and we ignored it for dwrite.h), but that's more important for mingw-w64 and they use our d3dx* headers (among others). Jacek
Re: [PATCH] Potential reference count races
On 10/29/12 01:25, Max TenEyck Woodbury wrote: On 10/28/2012 12:06 PM, Nikolay Sivov wrote: On 10/28/2012 17:44, Max TenEyck Woodbury wrote: On 10/28/2012 02:40 AM, Nikolay Sivov wrote: On 10/28/2012 04:59, m...@mtew.isa-geek.net wrote: From: Max TenEyck Woodbury m...@mtew.isa-geek.net I have been looking at the Microsoft COM and related documentations and noticed that they emphatically recommend using the Interlocked... functions when manipulating reference counts. I managed to set up a search that showed where many of the reference count updates occur and was somewhat surprised at how often this advice was not followed. It doesn't mean it always has to be followed. True in a limited sense, but there is a good reason behind their recommendation. Unless there is a good reason not to do so, this particular piece of advice should be followed. COM objects in wine follow this recommendation in general, even object itself is not thread safe. This doesn't mean however that you need this every time you have some kind of refcount of any sort. It may or may not be necessary every time, but it should be demonstrated that it is not necessary rather than assumed that it is not. This is a 'race condition' after all, and they are known to be rare and difficult to isolate. I think it is good practice to assume there could be a race problem rather than otherwise. No, it's a good practice to understand what you're changing instead of blindly assuming that everything with 'ref' in its name needs Interlocked* functions. You're patch even changes things in jscript that are not COM objecst, in a middle of code that is not thread safe anyway. It's documented that those parts of the code are not thread safe. Even worse, you were changing lines that even have a comment about no need for atomic operations (thanks for Michael, who predicted that someone may send patches like you). While I have not converted every reference count update to use the Interlocked... functions, this set of patches fixes a fair number of them. These are not associated with any particular bug report; they are simply a general precaution against operations that are known to be associated with race conditions. This precaution doesn't work in general. It's not enough to atomically update refcount to make an implementation thread safe. Also not everything is supposed to be thread safe in a first place. First, explain what does not have to be thread safe. Anything really, COM objects in particular if you were talking about them. I think you are talking about the apartment model here, which forces thread serialization. Despite that, the Microsoft documentation still recommends interlocked operations on reference counts... Just to make it clear: using Interlocked* functions in generic COM object is a good practice. But lack of it is not always a bug and you shouldn't change it without understanding what you are doing. I believe that application may try to use multiple threads anywhere, so everything that can be made thread safe, should be. No. What do you mean 'No'. That is an opinion, If you disagree, please explain why. See above. Changes like this: -for (i=0;ihowmuch;i++) +for (i=0;ihowmuch;++i) TRACE(notify at %d to %p\n, notify[i].dwOffset,notify[i].hEventNotify); are not helpful at all. The post increment and decrement operation are specified as saving the original value for use in the evaluation of the expression they are part of and modifying the underlying stored value. In expressions like this, that saved value is then discarded. The optimization phase of the compilation usually removes both the save and discard operations. Sure, but I don't think it's enough to justify such changes all over the place, in existing code. I agree that it is not enough to justify a separate set of patches, but as part of another set of changes, I think it is justified. After all, how else are examples of bad code going to be removed. This is not a bad coding. You're changing a lot of my code, which uses my coding style and this style is to use postfix incrementation. I'm not saying it's better of worse because it's not, so there is no reason to change it. Your argument about requirement for temporary storage is an example of bad way to think about the code. Jacek
Re: [PATCH 1/5] d3dx9_36: Implement D3DXFileCreate.
On 10/22/12 10:13 PM, Rico Schüller wrote: On 22.10.2012 21:27, Christian Costa wrote: +static HRESULT WINAPI ID3DXFileImpl_QueryInterface(ID3DXFile *iface, REFIID riid, LPVOID *ret_iface) +ID3DXFileImpl *This = impl_from_ID3DXFile(iface); +ID3DXFileImpl* object; Please be a bit more consistent across your patch... There are a lot more occurrences where you mix both usages. His usage seems natural when you think about using 'This' as analogy to 'this' in C++ implementation. That said, it's really a matter of taste, but such analogy feels natural when you work with COM. And by that logic, the patch is consistent. It uses 'object' as name of global function, which is not member of any class-like thing. Christian, there were some informal attempts to limit usage of stuff like LPVOID (and really any LP* types). It would be nice if you could avoid it in your patches. Jacek
Re: ntdll: Don't use strncasecmp for _strnicmp implementation
On 10/16/12 13:21, Jacek Caban wrote: How exactly? Do you know more details / urls? Does it return -n ... +n values? Like the memcmp optimization that caused mysql security issue? In that case my patch should work. From what I know following call crashes: strncasecmp(, , 1); It's probably a corner case for some optimizations. FWIW it was a known, already fixed, upstream glibc bug: http://sourceware.org/bugzilla/show_bug.cgi?id=14195 Jacek
Re: jscript: Don't release possible NULL pointer (coverity)
Hi André, On 10/18/12 00:14, André Hentschel wrote: CID 735258 --- dlls/jscript/regexp.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/dlls/jscript/regexp.c b/dlls/jscript/regexp.c index bdff445..71474c7 100644 --- a/dlls/jscript/regexp.c +++ b/dlls/jscript/regexp.c @@ -3378,7 +3378,9 @@ static HRESULT do_regexp_match_next(script_ctx_t *ctx, RegExpInstance *regexp, D last_match = jsstr_alloc_len(str, len); if(!last_match) return E_OUTOFMEMORY; -jsstr_release(ctx-last_match); + +if(ctx-last_match) +jsstr_release(ctx-last_match); ctx-last_match = last_match; ctx-last_match should always be a valid string. I guess coverity is confused by earlier compare to NULL, which is a leftover after string representation change. We should use jsstr_t here to avoid allocation anyway, I will send a patch. Thanks, Jacek
Re: ntdll: Don't use strncasecmp for _strnicmp implementation
On 10/16/12 13:08, Marcus Meissner wrote:
On Tue, Oct 16, 2012 at 12:38:51PM +0200, Jacek Caban wrote:
---
dlls/ntdll/string.c | 12 +++-
dlls/ntdll/tests/string.c | 33 +
2 files changed, 44 insertions(+), 1 deletions(-)
diff --git a/dlls/ntdll/string.c b/dlls/ntdll/string.c
index 716dbdf..288e910 100644
--- a/dlls/ntdll/string.c
+++ b/dlls/ntdll/string.c
@@ -254,7 +254,17 @@ int __cdecl _stricmp( LPCSTR str1, LPCSTR str2 )
*/
int __cdecl _strnicmp( LPCSTR str1, LPCSTR str2, size_t n )
{
-return strncasecmp( str1, str2, n );
+int ret = 0;
+
+/* 32-bit Windows return only -1,0,1 values */
+while(n--) {
+if(!*str1)
+return sizeof(void*) == 4 ? (*str2 ? -1 : 0) : -(unsigned
char)*str2;
+if((ret = tolower(*str1++) - tolower(*str2++)))
+return sizeof(void*) == 4 ? (ret 0 ? 1 : -1) : ret;
+}
Errm. Why not
int ret = strncasecmp( str1, str2, n );
if (ret 0 ) return -1;
if (ret 0 ) return 1;
return 0;
That wasn't the original reason for writing this patch. It seems like
some distros (well, at least some Gentoo installations) have broken
strncasecmp.
Jacek
Re: ntdll: Don't use strncasecmp for _strnicmp implementation
On 10/16/12 13:16, Marcus Meissner wrote:
On Tue, Oct 16, 2012 at 01:12:50PM +0200, Jacek Caban wrote:
On 10/16/12 13:08, Marcus Meissner wrote:
On Tue, Oct 16, 2012 at 12:38:51PM +0200, Jacek Caban wrote:
---
dlls/ntdll/string.c | 12 +++-
dlls/ntdll/tests/string.c | 33 +
2 files changed, 44 insertions(+), 1 deletions(-)
diff --git a/dlls/ntdll/string.c b/dlls/ntdll/string.c
index 716dbdf..288e910 100644
--- a/dlls/ntdll/string.c
+++ b/dlls/ntdll/string.c
@@ -254,7 +254,17 @@ int __cdecl _stricmp( LPCSTR str1, LPCSTR str2 )
*/
int __cdecl _strnicmp( LPCSTR str1, LPCSTR str2, size_t n )
{
-return strncasecmp( str1, str2, n );
+int ret = 0;
+
+/* 32-bit Windows return only -1,0,1 values */
+while(n--) {
+if(!*str1)
+return sizeof(void*) == 4 ? (*str2 ? -1 : 0) : -(unsigned
char)*str2;
+if((ret = tolower(*str1++) - tolower(*str2++)))
+return sizeof(void*) == 4 ? (ret 0 ? 1 : -1) : ret;
+}
Errm. Why not
int ret = strncasecmp( str1, str2, n );
if (ret 0 ) return -1;
if (ret 0 ) return 1;
return 0;
That wasn't the original reason for writing this patch. It seems like
some distros (well, at least some Gentoo installations) have broken
strncasecmp.
How exactly? Do you know more details / urls?
Does it return -n ... +n values? Like the memcmp optimization that caused
mysql security issue?
In that case my patch should work.
From what I know following call crashes:
strncasecmp(, , 1);
It's probably a corner case for some optimizations.
Jacek
Re: [PATCH 1/4] jscript: Use custom string container instead of BSTR.
On 10/11/12 19:12, Charles Davis wrote: On Oct 11, 2012, at 4:16 AM, Jacek Caban wrote: This patch alone makes SunSpider 0.9 17x faster. Seems to me that something is really wrong with our BSTR implementation if replacing it with a home-grown implementation speeds this up by a factor of 17. Not really, BSTR is just not the right tool in this case. BSTR is mostly just an allocator that also stores string length, but limits what you can do with it (eg. if you want to store substring as BSTR, a reallocation is needed). We used it in jscript mainly for historical reason. Strings handling is very important part of script engines and it's obvious that custom implementation is better. BTW, this patch is just beginning for string optimization. Things like atoms, lazy concatenation and dependent strings are still left to do. Cheers, Jacek
Re: [PATCH 1/4] jscript: Use custom string container instead of BSTR.
On 10/11/12 12:39, Henri Verbeet wrote:
On 11 October 2012 12:16, Jacek Caban ja...@codeweavers.com wrote:
+struct _jsstr_t {
+unsigned length_flags;
+unsigned ref;
+WCHAR str[1];
+};
It's probably much too late for jscript, and perhaps Wine in general,
but strictly speaking the _t suffix is reserved by POSIX.
I consider using it a matter of taste. It's often convenient to add
suffix so that the namespace is free for things like that:
mystryct_t *mystruct;
(and struct mystruct *mystruct; is a worse solution IMHO).
As a matter of fact, I was considering dropping _t suffix for jsstr_t
type (as well as jsval_t, which I introduced lately) mostly because
those types are so core for jscript that they deserve looking a bit like
base types. I didn't know about the suffix reservation for POSIX (well,
C99 also uses _t and it's not part of POSIX), that's a good point.
That said, I don't have strong opinion here for general practices in
Wine. I will reconsider dropping the suffix for a few types.
Jacek
Re: [PATCH] reg.exe: comparing the names of registry keys
On 10/04/12 08:03, Mieczyslaw Nalewaj wrote:
+static BOOL strings_equal(LPWSTR lpString1, const WCHAR *lpString2)
+{
+int cchCount;
Please avoid Hungarian style variable names. Also is seems like the
function not exactly compares strings, so something like is_subkey could
be more appropriate function name.
+if ((!lpString1 || !*lpString1) (!lpString2 || !*lpString2)) /* both
empty */
+return TRUE;
+if ( !lpString1 || !*lpString1 || !lpString2 || !*lpString2 ) /* one
empty */
+return FALSE;
How about simplifying it to:
if(!str1 || !*str1)
return !str2 || !*str2;
if(!str2 || !*str2)
return FALSE;
+cchCount = strlenW(lpString2);
+if (strlenW(lpString1)cchCount) /* too short */
+return FALSE;
+
+if (
(CompareStringW(CP_ACP,NORM_IGNORECASE,lpString1,cchCount,lpString2,cchCount)==CSTR_EQUAL)
+ ((lpString1[cchCount] == 0) || (lpString1[cchCount] ==
(WCHAR){'\\'})))
+return TRUE;
+else
+return FALSE;
This could be simplified:
len = strlenW(str2);
return !strncmpiW(str1, str2, len) (!str1[len] || str1[len] == '\\');
Cheers,
Jacek
Re: [PATCH 2/2] mshtml: Added ActiveX event binding test
On 10/04/12 15:02, Marvin wrote: 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 http://testbot.winehq.org/JobDetails.pl?Key=22013 Your paranoid android. Can someone please clear TestBot queue? Every time I send a series of two patches, it tries to test my previous series, which usually (like in this case) is already in Git. Jacek
Re: [PATCH 1/3] hhctrl.ocx: Fix removing a window from the help list when window creation fails.
On 10/02/12 23:02, Erich E. Hoover wrote: If a cleanup occurs because HTML Help window creation fails then list_remove() causes a crash, since the window was never added to the window list. The attached patch fixes this issue, allowing safe reloading of files (needed for part 2). This looks like a hack. It would be better to have always valid list entry (if this really can't be added to the list earlier, you may always initialize it by list_init) or make sure to not call ReleaseHelpViewer before HHInfo is fully initialized. Jacek
Re: [1/2] vbscript: Support vb* constants for message box buttons
Hi Nikolay, On 10/03/12 13:34, Nikolay Sivov wrote: Support vb* constants for message box buttons How about using MB_* constants from winuser.h instead of hardcoding them here? Other than that, the patch looks good to me. Jacek
Re: [PATCH] rpcrt4: wait_async_request: return error if we received an error
On 10/03/12 14:57, Marcus Meissner wrote: On Tue, Oct 02, 2012 at 01:19:37PM +0200, Jacek Caban wrote: Hi Marcus, On 10/01/12 23:00, Marcus Meissner wrote: Hi, Various coverity issues complain about user-after-free scenarios, all involving this code path. I strongly think if call_ret signals error, we also need to return an error condition to avoid the callers from proceeding as if nothing happened. Second reiteration with Jaceks comment applied Sorry for not catching this later, I was concentrated on your comment instead of the code, but the important thing is that true call_ret means success (and wininet doing request asynchronously is signalled as an error). It means that in this case we want to return RPC_S_OK. What is the exact problem? Ok, my fix was likely bad. Coverity is spotting use-after-free scenarios. CID 715805 I restored my login in Coverity and looked at the report. It seems to be false positive. async_data is reference counted structure and we'll never free it in mentioned wait_async_request because we still store the reference in caller. Cheers, Jacek
Re: [PATCH] rpcrt4: wait_async_request: return error if we received an error
Hi Marcus, On 10/01/12 23:00, Marcus Meissner wrote: Hi, Various coverity issues complain about user-after-free scenarios, all involving this code path. I strongly think if call_ret signals error, we also need to return an error condition to avoid the callers from proceeding as if nothing happened. Second reiteration with Jaceks comment applied Sorry for not catching this later, I was concentrated on your comment instead of the code, but the important thing is that true call_ret means success (and wininet doing request asynchronously is signalled as an error). It means that in this case we want to return RPC_S_OK. What is the exact problem? Jacek
Re: mshtml: Implement IHTMLFrameBase_put_name()
Hi Nikolay, On 09/30/12 20:36, Nikolay Sivov wrote: http://bugs.winehq.org/show_bug.cgi?id=31835 The code may be simple and obvious, but I'd still like to see a test. A simple call should be enough just to make sure it's executed when I test new Gecko builds. Jacek
Re: [PATCH] rpcrt4: wait_async_request: return error if we received an error
Hi Marcus,
On 09/29/12 11:44, Marcus Meissner wrote:
Hi,
Various coverity issues complain about user-after-free scenarios,
all involving this code path.
I strongly think if call_ret signals error, we also need to return
an error condition to avoid the callers from proceeding as if nothing
happened.
Agreed, thanks for catching it.
Ciao, Marcus
---
dlls/rpcrt4/rpc_transport.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/dlls/rpcrt4/rpc_transport.c b/dlls/rpcrt4/rpc_transport.c
index 686ce68..e572b78 100644
--- a/dlls/rpcrt4/rpc_transport.c
+++ b/dlls/rpcrt4/rpc_transport.c
@@ -1882,7 +1882,7 @@ static RPC_STATUS wait_async_request(RpcHttpAsyncData
*async_data, BOOL call_ret
if(call_ret) {
RpcHttpAsyncData_Release(async_data);
-return RPC_S_OK;
+return call_ret; /* The Http* status codes map into the RPC_S
namespace */
Not really, Http* APIs return BOOL and GetLastError() call is needed to
get error code. AFAIR, when I tested this code, most HTTP-related errors
were translated to RPC_S_SERVER_UNAVAILABLE by rpcrt4. That may be a
better choice here as well.
Jacek
Re: scrrun: Correct Invoke parameter
Hi Alistair,
On 09/21/12 14:12, Alistair Leslie-Hughes wrote:
Hi,
Changelog:
scrrun: Correct Invoke parameter
--- a/dlls/scrrun/dictionary.c
+++ b/dlls/scrrun/dictionary.c
@@ -153,7 +153,7 @@ static HRESULT WINAPI dictionary_Invoke(IDictionary *iface,
DISPID dispIdMember,
hr = get_typeinfo(IDictionary_tid, typeinfo);
if(SUCCEEDED(hr))
{
-hr = ITypeInfo_Invoke(typeinfo, iface, dispIdMember, wFlags,
+hr = ITypeInfo_Invoke(typeinfo, This, dispIdMember, wFlags,
pDispParams, pVarResult, pExcepInfo, puArgErr);
This really should be an interface, not an object. This-IDistionary_iface or
simply iface would be the right thing to do.
Jacek
Re: [PATCH 21/21] jscript: Pack jsval_t to 64-bit structure on i386
Hi Juan, On 09/17/12 22:42, Juan Lang wrote: Hi Jacek, + * that NaN value representation has 52 (almost) free bytes. You mean bits, yes? Yes, of course. While you're at it, + * jsval_t structure is used to represent JavaScript dynamicaly-typed values. dynamically is spelled with two l's. Thanks for catching those. I will fix it while resubmitting after I will fix exception value propagation. Thanks, Jacek
Re: [PATCH 21/21] jscript: Pack jsval_t to 64-bit structure on i386
Please ignore this last patch. It has one more dependency that I didn't send yet. On 09/17/12 15:21, Jacek Caban wrote: --- dlls/jscript/jsutils.c |4 +- dlls/jscript/jsval.h | 143 +-- 2 files changed, 114 insertions(+), 33 deletions(-)
Re: [PATCH 6/9] mshtml: Use first script host's GUID as default script guid
On 09/07/12 19:15, Alexandre Julliard wrote: Jacek Caban ja...@codeweavers.com writes: dlls/mshtml/htmlwindow.c |1 - dlls/mshtml/mshtml_private.h |1 - dlls/mshtml/script.c | 18 ++ dlls/mshtml/tests/vbtest.html | 15 +-- 4 files changed, 23 insertions(+), 12 deletions(-) It doesn't work here on 64-bit: ../../../../wine/tools/runtest -q -P wine -M mshtml.dll -T ../../.. -p mshtml_test.exe.so ../../../../wine/dlls/mshtml/tests/script.c touch script.ok script.c:620: Test failed: Lgot an exception script.c:620: Test failed: Lcounter = 3 make: *** [script.ok] Error 2 It required another vbscript patch to be applied first, sorry about that. Jacek
Re: mshtml: Added support for navigating to anchors with IDs containing '#'
On 09/10/12 12:19, Alexandre Julliard wrote:
Jacek Caban ja...@codeweavers.com writes:
---
dlls/mshtml/navigate.c | 42 +++---
dlls/mshtml/nsiface.idl | 11 +++
2 files changed, 50 insertions(+), 3 deletions(-)
It doesn't work here:
../../../tools/runtest -q -P wine -M mshtml.dll -T ../../.. -p
mshtml_test.exe.so htmldoc.c touch htmldoc.ok
htmldoc.c:4650: Test failed: unexpected riid
{5C8B21BC-EF6E-4599-A26F-FACC05B4ADBE}
make: *** [htmldoc.ok] Error 1
This looks like a bug in Gecko that doesn't set result to NULL when an
element is not found. Could you please confirm it with attached patch
(I'm interested in the error trace I've added).
Thanks,
Jacek
diff --git a/dlls/mshtml/navigate.c b/dlls/mshtml/navigate.c
index 91705dd..efd5a96 100644
--- a/dlls/mshtml/navigate.c
+++ b/dlls/mshtml/navigate.c
@@ -19,6 +19,7 @@
#include config.h
#include stdarg.h
+#include assert.h
#define COBJMACROS
#define NONAMELESSUNION
@@ -1925,10 +1926,13 @@ static HRESULT navigate_fragment(HTMLOuterWindow
*window, IUri *uri)
{
nsIDOMLocation *nslocation;
nsAString nsfrag_str;
+WCHAR *selector;
BSTR frag;
nsresult nsres;
HRESULT hres;
+const WCHAR selector_formatW[] =
{'a','[','i','d','=','','%','s','',']',0};
+
set_current_uri(window, uri);
nsres = nsIDOMWindow_GetLocation(window-nswindow, nslocation);
@@ -1945,12 +1949,45 @@ static HRESULT navigate_fragment(HTMLOuterWindow
*window, IUri *uri)
nsres = nsIDOMLocation_SetHash(nslocation, nsfrag_str);
nsAString_Finish(nsfrag_str);
nsIDOMLocation_Release(nslocation);
-SysFreeString(frag);
-if(NS_FAILED(nsres)) {
+if(NS_FAILED(nsres))
ERR(SetHash failed: %08x\n, nsres);
-return E_FAIL;
+
+/*
+ * IE supports scrolling to anchor elements with #hash ids (note that
'#' is part of the id),
+ * while Gecko scrolls only to elements with hash ids. We scroll the
page ourselves if
+ * a[id=#hash] element can be found.
+ */
+selector =
heap_alloc(sizeof(selector_formatW)+SysStringLen(frag)*sizeof(WCHAR));
+if(selector) {
+nsIDOMNodeSelector *node_selector;
+nsIDOMElement *nselem = (void*)0xdeadbeef;
+nsAString selector_str;
+
+nsres =
nsIDOMHTMLDocument_QueryInterface(window-base.inner_window-doc-nsdoc,
IID_nsIDOMNodeSelector,
+(void**)node_selector);
+assert(nsres == NS_OK);
+
+sprintfW(selector, selector_formatW, frag);
+nsAString_InitDepend(selector_str, selector);
+nsres = nsIDOMNodeSelector_QuerySelector(node_selector, selector_str,
nselem);
+ERR(nselem %p %08x\n, nselem, nsres);
+nsIDOMNodeSelector_Release(node_selector);
+nsAString_Finish(selector_str);
+heap_free(selector);
+if(NS_SUCCEEDED(nsres) nselem) {
+nsIDOMHTMLElement *html_elem;
+
+nsres = nsIDOMElement_QueryInterface(nselem,
IID_nsIDOMHTMLElement, (void**)html_elem);
+nsIDOMElement_Release(nselem);
+if(NS_SUCCEEDED(nsres)) {
+nsIDOMHTMLElement_ScrollIntoView(html_elem, TRUE, 1);
+nsIDOMHTMLElement_Release(html_elem);
+}
+}
}
+SysFreeString(frag);
+
if(window-doc_obj-doc_object_service) {
IDocObjectService_FireNavigateComplete2(window-doc_obj-doc_object_service,
window-base.IHTMLWindow2_iface, 0x10);
IDocObjectService_FireDocumentComplete(window-doc_obj-doc_object_service,
window-base.IHTMLWindow2_iface, 0);
diff --git a/dlls/mshtml/nsiface.idl b/dlls/mshtml/nsiface.idl
index d19d5df..a4202f0 100644
--- a/dlls/mshtml/nsiface.idl
+++ b/dlls/mshtml/nsiface.idl
@@ -2073,6 +2073,17 @@ interface nsIDOMHTMLStyleElement : nsIDOMHTMLElement
[
object,
+uuid(7cebc153-168a-416c-ba5a-56a8c2ddb2ec),
+local
+]
+interface nsIDOMNodeSelector : nsISupports
+{
+nsresult QuerySelector(const nsAString *selectors, nsIDOMElement
**_retval);
+nsresult QuerySelectorAll(const nsAString *selectors, nsIDOMNodeList
**_retval);
+}
+
+[
+object,
uuid(94928ab3-8b63-11d3-989d-001083010e9b),
local
]
Re: [PATCH 5/5] mshtml: Added VBScript as event attribute tests
Hi, Here is proper run: https://testbot.winehq.org/JobDetails.pl?Key=21409 Jacek
Re: Wine Gecko 1.8-beta1
On 09/02/12 20:52, Vincent Povirk wrote: On related note, I seriously consider dropping debug builds and replace them by unstripped release build or even just a separated package containing only debug symbols for the release build. No one has complained about the debugging symbols in Wine Mono yet. They are included mostly because I don't know how to get rid of them, and it seems silly to put effort into that given the debug symbols have proven useful so far. Are you suggesting shipping unstripped builds as default package version? That's not an option, xul.dll alone is over 700MB unstripped. Jacek
Re: [PATCH 1/6] mshtml: Moved getting frame by name to separated function
On 09/03/12 16:13, Alexandre Julliard wrote: Jacek Caban ja...@codeweavers.com writes: --- dlls/mshtml/htmlwindow.c | 127 ++ 1 files changed, 72 insertions(+), 55 deletions(-) gcc -m32 -c -I. -I. -I../../include -I../../include -D__WINESRC__ -DCOM_NO_WINDOWS_H -D_REENTRANT -fPIC -Wall -pipe -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers -Wstrict-prototypes -Wtype-limits -Wunused-but-set-parameter -Wwrite-strings -gdwarf-2 -gstrict-dwarf -fno-omit-frame-pointer -Wpointer-arith -Wlogical-op -Werror -I/usr/include/freetype2-g -O2 -o htmlwindow.o htmlwindow.c htmlwindow.c: In function ‘HTMLWindow2_item’: htmlwindow.c:416:13: error: variable ‘hres’ set but not used [-Werror=unused-but-set-variable] cc1: all warnings being treated as errors make: *** [htmlwindow.o] Error 1 My GCC didn't warn about it. I've resent a fixed version. Thanks, Jacek
Wine Gecko 1.8-beta1
Hi all,
It's time for another Gecko update. I've uploaded to SourceForge [1] new
Gecko builds. These are based on Firefox 16 beta. Other than tons of
changes inherited from Firefox and usual fixes to keep it working well
for us, it brings a few visible fixes:
- Proper noscript tag handling
- Synchronous ActiveX loading
- Possibility to properly call APIs requiring JSContext
- Possibility to implement HTMLStyleElement::styleSheet property
As usually, to test it you need the attached patch and Gecko build
placed in the right place [2]. Any help with testing is appreciated!
On related note, I seriously consider dropping debug builds and replace
them by unstripped release build or even just a separated package
containing only debug symbols for the release build. Here are my
thoughts about reasoning (in random order):
- People use debug build mostly to retrieve better backtraces.
Unstripped builds also give that ability.
- If someone really needs debug build, it's easy enough to build it
himself (and that's usually a good idea anyway)
- Debug builds are seriously different than release build, so it
occasionally exposes different set of problems, leading to invalid
assumptions. This is really something for developers rather than users.
- It costs my time spent builds and tests. And I consider it a serious
matter. Amount of work needed to keep Gecko relevant is constantly
growing. I try to mitigate it by doing long-term improvements upstream,
automating the process and improving my hardware, but that has its
limits. And the time spent on Gecko maintenance (as in pure maintenance,
not real improvements) costs Wine in terms of improvements I could make
in this time.
If someone has a reason to keep debug builds, let me know. Any comments
are welcomed.
Thanks,
Jacek
[1] http://sourceforge.net/projects/wine/files/Wine%20Gecko/1.8-beta1/
[2] http://wiki.winehq.org/Gecko
diff --git a/dlls/appwiz.cpl/addons.c b/dlls/appwiz.cpl/addons.c
index 82a6797..b569705 100644
--- a/dlls/appwiz.cpl/addons.c
+++ b/dlls/appwiz.cpl/addons.c
@@ -51,7 +51,7 @@
WINE_DEFAULT_DEBUG_CHANNEL(appwizcpl);
-#define GECKO_VERSION 1.7
+#define GECKO_VERSION 1.8-beta1
#ifdef __i386__
#define ARCH_STRING x86
diff --git a/dlls/mshtml/htmldoc.c b/dlls/mshtml/htmldoc.c
index 86615df..5ac8e4e 100644
--- a/dlls/mshtml/htmldoc.c
+++ b/dlls/mshtml/htmldoc.c
@@ -1022,24 +1022,12 @@ static HRESULT WINAPI
HTMLDocument_createElement(IHTMLDocument2 *iface, BSTR eTa
IHTMLElement **newElem)
{
HTMLDocument *This = impl_from_IHTMLDocument2(iface);
-HTMLDocumentNode *doc_node;
-nsIDOMHTMLElement *nselem;
HTMLElement *elem;
HRESULT hres;
TRACE((%p)-(%s %p)\n, This, debugstr_w(eTag), newElem);
-/* Use owner doc if called on document fragment */
-doc_node = This-doc_node;
-if(!doc_node-nsdoc)
-doc_node = doc_node-node.doc;
-
-hres = create_nselem(doc_node, eTag, nselem);
-if(FAILED(hres))
-return hres;
-
-hres = HTMLElement_Create(doc_node, (nsIDOMNode*)nselem, TRUE, elem);
-nsIDOMHTMLElement_Release(nselem);
+hres = create_element(This-doc_node, eTag, elem);
if(FAILED(hres))
return hres;
@@ -1443,11 +1431,55 @@ static HRESULT WINAPI
HTMLDocument_createStyleSheet(IHTMLDocument2 *iface, BSTR
LONG lIndex, IHTMLStyleSheet
**ppnewStyleSheet)
{
HTMLDocument *This = impl_from_IHTMLDocument2(iface);
+nsIDOMHTMLHeadElement *head_elem;
+IHTMLStyleElement *style_elem;
+HTMLElement *elem;
+nsresult nsres;
+HRESULT hres;
-FIXME((%p)-(%s %d %p) semi-stub\n, This, debugstr_w(bstrHref), lIndex,
ppnewStyleSheet);
+static const WCHAR styleW[] = {'s','t','y','l','e',0};
-*ppnewStyleSheet = HTMLStyleSheet_Create(NULL);
-return S_OK;
+TRACE((%p)-(%s %d %p)\n, This, debugstr_w(bstrHref), lIndex,
ppnewStyleSheet);
+
+if(!This-doc_node-nsdoc) {
+FIXME(not a real doc object\n);
+return E_NOTIMPL;
+}
+
+if(lIndex != -1)
+FIXME(Unsupported lIndex %d\n, lIndex);
+
+if(bstrHref) {
+FIXME(semi-stub for href %s\n, debugstr_w(bstrHref));
+*ppnewStyleSheet = HTMLStyleSheet_Create(NULL);
+return S_OK;
+}
+
+hres = create_element(This-doc_node, styleW, elem);
+if(FAILED(hres))
+return hres;
+
+nsres = nsIDOMHTMLDocument_GetHead(This-doc_node-nsdoc, head_elem);
+if(NS_SUCCEEDED(nsres)) {
+nsIDOMNode *tmp_node;
+
+nsres = nsIDOMHTMLHeadElement_AppendChild(head_elem,
(nsIDOMNode*)elem-nselem, tmp_node);
+nsIDOMHTMLHeadElement_Release(head_elem);
+if(NS_SUCCEEDED(nsres) tmp_node)
+nsIDOMNode_Release(tmp_node);
+}
+if(NS_FAILED(nsres)) {
+IHTMLElement_Release(elem-IHTMLElement_iface);
+return E_FAIL;
+}
+
+hres = IHTMLElement_QueryInterface(elem-IHTMLElement_iface,
