On Tue, 21 Jan 2025 14:33:57 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:

>> Julian Waters has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains eight commits:
>> 
>>  - Merge branch 'openjdk:master' into accessibility
>>  - Cast to void in AccessBridgeCalls.c
>>  - static_cast to void in jaccessinspector.cpp
>>  - Formatting changes in AccessBridgeEventHandler.cpp
>>  - Merge branch 'master' into accessibility
>>  - Remove now unused result
>>  - Merge branch 'master' into accessibility
>>  - 8342870
>
> src/jdk.accessibility/windows/native/jaccesswalker/jaccesswalker.cpp line 261:
> 
>> 259:         GetClientRect(hWnd, &rcClient);
>> 260:      // hwndEdit =
>> 261:                    CreateWindow("Edit",
> 
> The edit control is created visible and it's resized to cover the entire 
> client area.
> 
> The edit control is used… the text is updated in the [`displayAndLog` 
> function](https://github.com/openjdk/jdk/blob/c38417a86e27f047715cfd9a98770387d994a512/src/jdk.accessibility/windows/native/toolscommon/AccessInfo.cpp#L60).
> 
> Since the edit control is a child window, it's automatically destroyed when 
> the parent window is destroyed.
> 
> Therefore, there's no reason to store the handle in a variable, and I suggest 
> removing the `hwndEdit` variable.
> 
> Moreover, the comment for `hwndEdit`—`// handle of tree-view control`—is 
> confusing because it doesn't store a handle to a TreeView control.

I wonder if this falls into the category of "Can be used in a debugger" like 
Alex mentioned above, if not I'll remove it

> src/jdk.accessibility/windows/native/jaccesswalker/jaccesswalker.cpp line 561:
> 
>> 559:         tvis.item = tvi;
>> 560: 
>> 561:         /* HTREEITEM treeNodeItem = */ TreeView_InsertItem(treeWnd, 
>> &tvis);
> 
> Since it's unused, I'd rather remove the variable altogether.
> 
> Does the handle have any other usage except for checking whether an error 
> occurred?

I'm not sure which handle you're referring to, you mean treeNodeItem? It 
doesn't seem to be used for checking if an error occurred

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21656#discussion_r1926401417
PR Review Comment: https://git.openjdk.org/jdk/pull/21656#discussion_r1926400915

Reply via email to