RFR: JDK-8311113: Remove invalid pointer cast and clean up setLabel() in awt_MenuItem.cpp
Alexey Ivanov
aivanov at openjdk.org
Wed Aug 30 21:16:35 UTC 2023
On Wed, 30 Aug 2023 18:42:20 GMT, Phil Race <prr at openjdk.org> wrote:
> > > Please double-check how the external tools like jaws/narrator will work after this change, will they be able to read the correct content of the menu item?
> >
> >
> > The updating of the label works correctly since the application handles the drawing/updating of the string as specified by the `mii.fType = MFT_OWNERDRAW` without requiring to set `dwTypeData`. To double-check ran JAWS and it works as expected & reads out the new label name when updated.
>
> Hmm, so where does JAWS get that info if we no longer set it ? It isn't scraping pixels and doing OCR.
Isn't JAWS using Accessible interfaces?
@prrace The string has never been set because AWT menus aren't of `MFT_STRING` type.
When a new menu item is added to menu, we use [`AppendMenuW`](https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-appendmenuw):
https://github.com/openjdk/jdk/blob/3eac8905aee6edecbebcc12a41300d3ce176fbff/src/java.desktop/windows/native/libawt/windows/awt_Menu.cpp#L214-L221
The last parameter passed to the `AppendMenuW` is `itemInfo`, its value is either `this` or a pointer `AwtMenuItem*` passed to the `AddItem` method.
So, the string value of a menu item is never set if `setLabel` is never called.
The code above confusingly “sets” the `MF_STRING` flag:
https://github.com/openjdk/jdk/blob/3eac8905aee6edecbebcc12a41300d3ce176fbff/src/java.desktop/windows/native/libawt/windows/awt_Menu.cpp#L212-L213
which is overridden by the `MF_OWNERDRAW`.
Because the value of `MF_STRING` is 0, it cannot be combined with `MF_BITMAP` or `MF_OWNERDRAW`. Thus the menu is a regular string menu if and only if neither `MF_BITMAP` or `MF_OWNERDRAW` are set, which is **never true** for AWT menus because `MF_OWNERDRAW` is always set.
The above holds true for the flags in `MENUITEMINFOW` structure.
> Seems like a lot of changes here to silence a warning we don't even get.
Even though we don't get this warning, the value set to `mii.dwTypeData` has no meaning, neither to the OS nor to AWT.
I strongly think that we should remove the code which copies the string label because it's never used.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/15276#issuecomment-1699846911
More information about the client-libs-dev
mailing list