[PATCH 2/2] conhost: Popup Attributes should match Character Attributes when a new screen buffer is created

[Jacek Caban]

On 7/25/21 7:42 AM, Hugh McMaster wrote:
> Hi Jacek,
>
> On Fri, 23 Jul 2021 at 22:24, Jacek Caban wrote:
>> Hi Hugh,
>>
>> On 7/22/21 3:36 PM, Hugh McMaster wrote:
>>> @@ -104,6 +104,7 @@ static struct screen_buffer *create_screen_buffer( struct console *console, int
>>>            screen_buffer->win.right  = width - 1;
>>>            screen_buffer->win.bottom = height - 1;
>>>            screen_buffer->attr       = FOREGROUND_BLUE|FOREGROUND_GREEN|FOREGROUND_RED;
>>> +        screen_buffer->popup_attr = screen_buffer->attr;
>>
>> This part doesn't seem right. For windowed consoles this part is
>> overwritten when we read the config, but it's not the case for pseudo
>> consoles (or when we use Unix console). This breaks the attached test.
>> diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c
>> index 6ee00aa49a6..6e53355f6e5 100644
>> --- a/dlls/kernel32/tests/console.c
>> +++ b/dlls/kernel32/tests/console.c
>> @@ -4352,6 +4352,7 @@ static void test_AllocConsole(void)
>>
>> static void test_pseudo_console_child(HANDLE input, HANDLE output)
>> {
>> +    CONSOLE_SCREEN_BUFFER_INFOEX sb_infoex;
>>      CONSOLE_SCREEN_BUFFER_INFO sb_info;
>>      CONSOLE_CURSOR_INFO cursor_info;
>>      DWORD mode;
>> @@ -4402,6 +4403,13 @@ static void test_pseudo_console_child(HANDLE input, HANDLE output)
>>      ok(sb_info.srWindow.Bottom == 29, "srWindow.Bottom = %u\n", sb_info.srWindow.Bottom);
>>      ok(sb_info.dwMaximumWindowSize.X == 40, "dwMaximumWindowSize.X = %u\n", sb_info.dwMaximumWindowSize.X);
>>      ok(sb_info.dwMaximumWindowSize.Y == 30, "dwMaximumWindowSize.Y = %u\n", sb_info.dwMaximumWindowSize.Y);
>> +    ok(sb_info.wAttributes == 0x7, "wAttributes = %#x\n", sb_info.wAttributes);
>> +
>> +    sb_infoex.cbSize = sizeof(sb_infoex);
>> +    ret = GetConsoleScreenBufferInfoEx(output, &sb_infoex);
>> +    ok(ret, "GetConsoleScreenBufferInfo failed: %u\n", GetLastError());
>> +    ok(sb_infoex.wAttributes == 0x7, "wAttributes = %#x\n", sb_infoex.wAttributes);
>> +    ok(sb_infoex.wPopupAttributes == 0xf5, "wPopupAttributes = %#x\n", sb_infoex.wPopupAttributes);
>>
>>      ret = GetConsoleCursorInfo(output, &cursor_info);
>>      ok(ret, "GetConsoleCursorInfo failed: %u\n", GetLastError());
> What is this testing? Currently, it just tells us what the color
> attributes and popup attributes are. There's nothing unexpected here.
>
> I ran a test using the --pseudo-console flag while creating a new
> screen buffer [1] and found the popup attributes take on the same
> value as the color attributes, in line with the other tests and patch.
>
> How should I be testing this?
>

Just run the test with no arguments, the test will create a pseudo 
console launch a child process attached to the pseudo console. It's a 
bit tricky, because you will not see printfs from failures on Wine, but 
you will still see it failing or not. The test shows that a fresh new 
pseudo console does not have default popup attributes equal to 
attributes (like in your patch).


Thanks,

Jacek