[PATCH] comctl32/tests: Fix propsheet template buffer size test on RTL locales
Fixes test failures on Arabic and Hebrew Windows. For some reason, on these locales Windows uses a different formula for allocating the template buffer, allocating a little more than 2 times the resource size on Hebrew and a little less on Arabic. Because we don't know how exactly the buffer size is determined on these locales, just accept a size near 2 times the resource size. Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/comctl32/tests/propsheet.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/comctl32/tests/propsheet.c b/dlls/comctl32/tests/propsheet.c index a3f910d2d5..68dce519bb 100644 --- a/dlls/comctl32/tests/propsheet.c +++ b/dlls/comctl32/tests/propsheet.c @@ -58,16 +58,21 @@ static int CALLBACK sheet_callback(HWND hwnd, UINT msg, LPARAM lparam) case PSCB_PRECREATE: { HMODULE module = GetModuleHandleA("comctl32.dll"); + HMODULE kernel32 = GetModuleHandleA("kernel32.dll"); + LANGID (WINAPI *pGetThreadUILanguage)(void) = (void *)GetProcAddress(kernel32, "GetThreadUILanguage"); DWORD size, buffer_size; HRSRC hrsrc; + int reading_layout; hrsrc = FindResourceA(module, MAKEINTRESOURCEA(1006 /* IDD_PROPSHEET */), (LPSTR)RT_DIALOG); size = SizeofResource(module, hrsrc); ok(size != 0, "Failed to get size of propsheet dialog resource\n"); buffer_size = HeapSize(GetProcessHeap(), 0, (void *)lparam); - ok(buffer_size == 2 * size, "Unexpected template buffer size %u, resource size %u\n", - buffer_size, size); + GetLocaleInfoA(pGetThreadUILanguage ? pGetThreadUILanguage() : GetUserDefaultUILanguage(), + LOCALE_IREADINGLAYOUT | LOCALE_RETURN_NUMBER, (void *)&reading_layout, sizeof(reading_layout)); + ok(reading_layout == 2 /* RTL */ ? abs(buffer_size - 2 * size) <= 32 : buffer_size == 2 * size, + "Unexpected template buffer size %u, resource size %u\n", buffer_size, size); break; } case PSCB_INITIALIZED: -- 2.15.0
On 13.11.2017 8:13, Alex Henrie wrote:
Fixes test failures on Arabic and Hebrew Windows. For some reason, on these locales Windows uses a different formula for allocating the template buffer, allocating a little more than 2 times the resource size on Hebrew and a little less on Arabic. Because we don't know how exactly the buffer size is determined on these locales, just accept a size near 2 times the resource size.
Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/comctl32/tests/propsheet.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/dlls/comctl32/tests/propsheet.c b/dlls/comctl32/tests/propsheet.c index a3f910d2d5..68dce519bb 100644 --- a/dlls/comctl32/tests/propsheet.c +++ b/dlls/comctl32/tests/propsheet.c @@ -58,16 +58,21 @@ static int CALLBACK sheet_callback(HWND hwnd, UINT msg, LPARAM lparam) case PSCB_PRECREATE: { HMODULE module = GetModuleHandleA("comctl32.dll"); + HMODULE kernel32 = GetModuleHandleA("kernel32.dll"); + LANGID (WINAPI *pGetThreadUILanguage)(void) = (void *)GetProcAddress(kernel32, "GetThreadUILanguage"); DWORD size, buffer_size; HRSRC hrsrc; + int reading_layout;
hrsrc = FindResourceA(module, MAKEINTRESOURCEA(1006 /* IDD_PROPSHEET */), (LPSTR)RT_DIALOG); size = SizeofResource(module, hrsrc); ok(size != 0, "Failed to get size of propsheet dialog resource\n"); buffer_size = HeapSize(GetProcessHeap(), 0, (void *)lparam); - ok(buffer_size == 2 * size, "Unexpected template buffer size %u, resource size %u\n", - buffer_size, size); + GetLocaleInfoA(pGetThreadUILanguage ? pGetThreadUILanguage() : GetUserDefaultUILanguage(), + LOCALE_IREADINGLAYOUT | LOCALE_RETURN_NUMBER, (void *)&reading_layout, sizeof(reading_layout)); + ok(reading_layout == 2 /* RTL */ ? abs(buffer_size - 2 * size) <= 32 : buffer_size == 2 * size, + "Unexpected template buffer size %u, resource size %u\n", buffer_size, size); break; } case PSCB_INITIALIZED:
Is it possible we're picking wrong resource with FindResource() here? Maybe we should be using explicit language instead of a neutral one, so --- propsheet.c:69: Test failed: Unexpected template buffer size 508, resource size 270 --- could mean we were supposed to pick up a different resource of size 254. P.S. GetLocaleInfo() takes LCID not LANGID, and you can use special constants like LOCALE_USER_DEFAULT too.
2017-11-14 0:40 GMT-07:00 Nikolay Sivov <bunglehead(a)gmail.com>:
Is it possible we're picking wrong resource with FindResource() here? Maybe we should be using explicit language instead of a neutral one, so
--- propsheet.c:69: Test failed: Unexpected template buffer size 508, resource size 270 ---
could mean we were supposed to pick up a different resource of size 254.
I tried FindResourceExA on Hebrew Windows 10 with all of the following, and got a resource of size 250 every time: MAKELANGID(LANG_NEUTRAL, SUBLANG_NEUTRAL) MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT) MAKELANGID(LANG_NEUTRAL, SUBLANG_SYS_DEFAULT) MAKELANGID(LANG_NEUTRAL, SUBLANG_CUSTOM_DEFAULT) MAKELANGID(LANG_NEUTRAL, SUBLANG_CUSTOM_UNSPECIFIED) MAKELANGID(LANG_NEUTRAL, SUBLANG_UI_CUSTOM_DEFAULT) MAKELANGID(LANG_HEBREW, SUBLANG_HEBREW_ISRAEL) Interestingly, 508 is exactly 2 times the US English resource size. Maybe the RTL locales base their buffer size on the US English resource? I checked and other languages such as German have different resource and buffer sizes from English. I also checked Kurdish, Persian, Punjabi, Urdu, and Uyghur Windows 10, and resource sizes were the same for both the localized resource and the US English resource. So, I couldn't tell whether they compute buffer size based on the English resource or based on their own resource.
P.S. GetLocaleInfo() takes LCID not LANGID, and you can use special constants like LOCALE_USER_DEFAULT too.
OK, LOCALE_USER_DEFAULT should be fine. Out of curiosity, what is the correct way to specify a LCID? Do I have to say something like MAKELCID(MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US), 0)? More importantly, how do you think these tests should be written? Should I use the US English resource size to determine the correct buffer size on RTL locales, or should I just continue accepting sizes within 32 bytes of expected? Also, should the different code path be for all RTL locales, or just for Arabic and Hebrew specifically? -Alex
On 11/15/2017 09:05 AM, Alex Henrie wrote:
2017-11-14 0:40 GMT-07:00 Nikolay Sivov <bunglehead(a)gmail.com>:
Is it possible we're picking wrong resource with FindResource() here? Maybe we should be using explicit language instead of a neutral one, so
--- propsheet.c:69: Test failed: Unexpected template buffer size 508, resource size 270 ---
could mean we were supposed to pick up a different resource of size 254. I tried FindResourceExA on Hebrew Windows 10 with all of the following, and got a resource of size 250 every time:
MAKELANGID(LANG_NEUTRAL, SUBLANG_NEUTRAL) MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT) MAKELANGID(LANG_NEUTRAL, SUBLANG_SYS_DEFAULT) MAKELANGID(LANG_NEUTRAL, SUBLANG_CUSTOM_DEFAULT) MAKELANGID(LANG_NEUTRAL, SUBLANG_CUSTOM_UNSPECIFIED) MAKELANGID(LANG_NEUTRAL, SUBLANG_UI_CUSTOM_DEFAULT) MAKELANGID(LANG_HEBREW, SUBLANG_HEBREW_ISRAEL)
Interestingly, 508 is exactly 2 times the US English resource size. Maybe the RTL locales base their buffer size on the US English resource? I checked and other languages such as German have different resource and buffer sizes from English.
I also checked Kurdish, Persian, Punjabi, Urdu, and Uyghur Windows 10, and resource sizes were the same for both the localized resource and the US English resource. So, I couldn't tell whether they compute buffer size based on the English resource or based on their own resource. That's confusing, how can they all be the same if we have a failing test? What's picked up as neutral resource, that's a question I guess. Also it sounds strange that localized strings don't affect template size, but maybe strings are stored separately. If regardless of locale it's always using US English size, that's what we should use too, still it's interesting where size 270 comes from.
P.S. GetLocaleInfo() takes LCID not LANGID, and you can use special constants like LOCALE_USER_DEFAULT too. OK, LOCALE_USER_DEFAULT should be fine. Out of curiosity, what is the correct way to specify a LCID? Do I have to say something like MAKELCID(MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US), 0)?
More importantly, how do you think these tests should be written? Should I use the US English resource size to determine the correct buffer size on RTL locales, or should I just continue accepting sizes within 32 bytes of expected? Also, should the different code path be for all RTL locales, or just for Arabic and Hebrew specifically? Ideally we should test against actual exact size, using single path, and not testing locale properties.
-Alex
2017-11-14 23:21 GMT-07:00 Nikolay Sivov <nsivov(a)codeweavers.com>:
On 11/15/2017 09:05 AM, Alex Henrie wrote:
2017-11-14 0:40 GMT-07:00 Nikolay Sivov <bunglehead(a)gmail.com>:
Is it possible we're picking wrong resource with FindResource() here? Maybe we should be using explicit language instead of a neutral one, so
--- propsheet.c:69: Test failed: Unexpected template buffer size 508, resource size 270 ---
could mean we were supposed to pick up a different resource of size 254.
I tried FindResourceExA on Hebrew Windows 10 with all of the following, and got a resource of size 250 every time:
MAKELANGID(LANG_NEUTRAL, SUBLANG_NEUTRAL) MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT) MAKELANGID(LANG_NEUTRAL, SUBLANG_SYS_DEFAULT) MAKELANGID(LANG_NEUTRAL, SUBLANG_CUSTOM_DEFAULT) MAKELANGID(LANG_NEUTRAL, SUBLANG_CUSTOM_UNSPECIFIED) MAKELANGID(LANG_NEUTRAL, SUBLANG_UI_CUSTOM_DEFAULT) MAKELANGID(LANG_HEBREW, SUBLANG_HEBREW_ISRAEL)
Interestingly, 508 is exactly 2 times the US English resource size. Maybe the RTL locales base their buffer size on the US English resource? I checked and other languages such as German have different resource and buffer sizes from English.
I also checked Kurdish, Persian, Punjabi, Urdu, and Uyghur Windows 10, and resource sizes were the same for both the localized resource and the US English resource. So, I couldn't tell whether they compute buffer size based on the English resource or based on their own resource.
That's confusing, how can they all be the same if we have a failing test? What's picked up as neutral resource, that's a question I guess. Also it sounds strange that localized strings don't affect template size, but maybe strings are stored separately. If regardless of locale it's always using US English size, that's what we should use too, still it's interesting where size 270 comes from.
I used Hebrew Windows for testing because it's simpler than Arabic: Windows only recognizes one Hebrew dialect. English: Resource size 254, buffer size 508 Arabic: Resource size 270, buffer size 508 Hebrew: Resource size 250, buffer size 508 German: Resource size 266, buffer size 532
P.S. GetLocaleInfo() takes LCID not LANGID, and you can use special constants like LOCALE_USER_DEFAULT too.
OK, LOCALE_USER_DEFAULT should be fine. Out of curiosity, what is the correct way to specify a LCID? Do I have to say something like MAKELCID(MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US), 0)?
More importantly, how do you think these tests should be written? Should I use the US English resource size to determine the correct buffer size on RTL locales, or should I just continue accepting sizes within 32 bytes of expected? Also, should the different code path be for all RTL locales, or just for Arabic and Hebrew specifically?
Ideally we should test against actual exact size, using single path, and not testing locale properties.
Considering the numbers I gave above, how can we have only a single path? -Alex
Alex Henrie <alexhenrie24(a)gmail.com> writes:
I used Hebrew Windows for testing because it's simpler than Arabic: Windows only recognizes one Hebrew dialect.
English: Resource size 254, buffer size 508 Arabic: Resource size 270, buffer size 508 Hebrew: Resource size 250, buffer size 508 German: Resource size 266, buffer size 532
Is it actually using the Hebrew resource though? Maybe it needs the propsheet pages to be RTL or something like that. -- Alexandre Julliard julliard(a)winehq.org
2017-11-15 11:46 GMT-07:00 Alexandre Julliard <julliard(a)winehq.org>:
Alex Henrie <alexhenrie24(a)gmail.com> writes:
I used Hebrew Windows for testing because it's simpler than Arabic: Windows only recognizes one Hebrew dialect.
English: Resource size 254, buffer size 508 Arabic: Resource size 270, buffer size 508 Hebrew: Resource size 250, buffer size 508 German: Resource size 266, buffer size 532
Is it actually using the Hebrew resource though? Maybe it needs the propsheet pages to be RTL or something like that.
Interesting, I think you're right: If I call SetProcessDefaultLayout(LAYOUT_RTL) at the beginning of the tests then the buffer size tests pass. This suggests three possible solutions: 1. Call SetProcessDefaultLayout(LAYOUT_RTL) before any of the tests run, and change test_buttons to expect the buttons to be in reverse order if on an RTL locale. 2. Call SetProcessDefaultLayout(LAYOUT_RTL) before each buffer size test, and restore the original setting after each of those tests finishes. 3. Check for an RTL locale in sheet_callback and expect a US English resource if the locale is RTL but the test program is LTR. Which of these solutions is preferred? -Alex
Alex Henrie <alexhenrie24(a)gmail.com> writes:
2017-11-15 11:46 GMT-07:00 Alexandre Julliard <julliard(a)winehq.org>:
Alex Henrie <alexhenrie24(a)gmail.com> writes:
I used Hebrew Windows for testing because it's simpler than Arabic: Windows only recognizes one Hebrew dialect.
English: Resource size 254, buffer size 508 Arabic: Resource size 270, buffer size 508 Hebrew: Resource size 250, buffer size 508 German: Resource size 266, buffer size 532
Is it actually using the Hebrew resource though? Maybe it needs the propsheet pages to be RTL or something like that.
Interesting, I think you're right: If I call SetProcessDefaultLayout(LAYOUT_RTL) at the beginning of the tests then the buffer size tests pass. This suggests three possible solutions:
1. Call SetProcessDefaultLayout(LAYOUT_RTL) before any of the tests run, and change test_buttons to expect the buttons to be in reverse order if on an RTL locale.
This one is probably the best, it's the one nearest to normal application usage. -- Alexandre Julliard julliard(a)winehq.org
participants (4)
-
Alex Henrie -
Alexandre Julliard -
Nikolay Sivov -
Nikolay Sivov