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