<AWT Dev> [7] Review request for 6891269: non-compliance with Section 508 using 1.6.0_18

[Artem Ananiev]

Hi, Vlad,

On 6/9/2012 6:00 PM, Vladislav Karnaukhov wrote:
> Hello Artem,
>
> please see comment inline.
>
> Regards,
> - Vlad
>
> On 04.06.12 17:48, Artem Ananiev wrote:
>> Hi, Vladislav,
>>
>> some random comments:
>>
>> 1. From the bug description, it's not clear when the problem is
>> reproducible. Does caret size become 1x1 pixels on windows theme changes?
>>
>> 2. AwtComponent class already has WmSettingsChanged() callback, so you
>> can place ::SystemParametersInfo() there, not into AwtToolkit.
>>
>> 3. Does the problem only manifests in text areas? What about text fields?
>>
>> 4. Could you describe why UpdateCaret() is required in WmForwardChar()
>> and in HandleEvent()? I would expect ::CreateCaret() in WmSetFocus(),
>> ::DestroyCaret() in WmKillFocus(), and re-create caret in
>> WmSettingChanged() should be enough.
> Seems like AwtTextArea doesn't receive WM_SETTINGCHANGE message.
> WM_SETFOCUS and WM_KILLFOCUS are handled correctly, but I can't update
> actual width of the caret.

according to MSDN, WM_SETTINGCHANGE is only sent to top-level windows. 
That's why AwtTextArea::WmSettingChange() is never called. Given that, 
your initial approach to update caret width in AwtToolkit sounds 
reasonable. I would just move m_dwCaretWidth from AwtToolkit to AwtTextArea.

 From the other side, it's pretty strange that RichEdit, which is used 
in AwtTextArea, doesn't handle caret changes automatically... Probably, 
that's because we consume the message in AwtWindow (see "return 
mrConsume" in AwtWindow::WmSettingChange()). Have you tried looking into 
that direction?

Thanks,

Artem

> I've put caret width update logic into AwtComponent::WmSettingChange,
> but seems like it's updated incorrectly (or my .pdb files are incorrect).
>
> If you would help me understand why AwtTextArea doesn't receive
> WM_SETTINGCHANGED, it would be most useful. Is this message decorated
> somehow by AwtComponent?
>
>>
>>
>> 5. If we introduce UpdateCaret() method, it should be placed either to
>> AwtTextArea or to AwtTextComponent, there is no need to have this
>> method in AwtComponent.
>>
>> Thanks,
>>
>> Artem
>>
>> On 6/1/2012 6:28 PM, Vladislav Karnaukhov wrote:
>>> Hello,
>>>
>>> please review a fix for the following issue: 6891269 non-compliance with
>>> Section 508 using 1.6.0_18
>>>
>>> http://monaco.sfbay.sun.com/detail.jsf?cr=6891269
>>>
>>> Please find webrev here:
>>> http://cr.openjdk.java.net/~alexp/6891269/webrev.00/
>>>
>>> This is a forward-port from JDK6. The fix introduces support for Windows
>>> Accessibility/Ease Of Access to TextArea control.
>>>
>>> Regards,
>>> - Vlad
>