On Fri, 25 Aug 2023 19:18:45 GMT, Harshitha Onkar <[email protected]> wrote:

>> In awt_MenuItem.cpp (712,22): ` mii.dwTypeData = (LPTSTR)(*sb)`  produces 
>> invalid pointer cast warning when complied on clang and moreover this is a 
>> no-op.  
>> 
>> `mii.dwTypeData` is used only when **MIIM_STRING** flag is set in the fMask 
>> (as per 
>> [Docs](https://learn.microsoft.com/en-us/windows/win32/api/winuser/ns-winuser-menuiteminfoa)),
>>  which is not the case in JDK 
>> [Ln#705](https://github.com/openjdk/jdk/blob/e56d3bc2dab3d32453b6eda66e8434953c436084/src/java.desktop/windows/native/libawt/windows/awt_MenuItem.cpp#L706).
>>  Hence the assignment ` mii.dwTypeData = (LPTSTR)(*sb)`  is not required and 
>> so is the label parameter. Additionally necessary cleanup is done at the 
>> following places -
>> 
>> - WMenuItemPeer.java - to the native function call
>> - awt_MenuItem.cpp -  `WMenuItemPeer__1setLabel() ,_SetLabel(), SetLabel()`
>> - awt_MenuItem.h
>> 
>> Added a test which checks setLabel() functionality on Menu, MenuItem and 
>> PopupMenu.
>
> Harshitha Onkar has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   removed robot.waitForIdle() calls

[As for 
**`MIIM_TYPE`**](https://github.com/openjdk/jdk/pull/15276#discussion_r1296188625):
 the flag sets or retrieves both `fType` and `dwTypeData` members. In Windows 
95, the `MENUITEMINFO` structure was shorter, the `dwTypeData` field was a 
string if menu item is of type `MFT_STRING` (this implies neither MFT_BITMAP 
nor MFT_OWNERDRAW are set), it was an `HBITMAP` if the type is `MFT_BITMAP`. If 
the menu type is `MFT_OWNERDRAW`, <q 
cite="https://learn.microsoft.com/en-us/windows/win32/api/winuser/ns-winuser-menuiteminfow";>the
 dwTypeData member contains an application-defined value.</q> However, as we 
can see, it's not preserved.

Windows 98 added new members to the `MENUITEMINFO` structure. It allowed 
combining both `MFT_STRING` and `MFT_BITMAP` at the same time, which allows 
displaying a bitmap icon next to a string label. This feature is actively used 
by Windows to these days in the window system menu which you can access by 
left-clicking the app icon in the window title bar or by right-clicking the 
window title bar: the icons that you see next to Restore, Minimize, Maximize, 
and Close are displayed by combining `MFT_STRING | MFT_BITMAP`.

At the same time, `MIIM_TYPE` was replaced by more specific mask flags 
`MIIM_BITMAP`, `MIIM_FTYPE`, and `MIIM_STRING` which set or retrieve a 
designated member of the `MENUITEMINFO` structure.

This is why we should switch to using **`MIIM_FTYPE`** mask flag which clearly 
indicates neither `dwTypeData` nor `cch` are used.

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

PR Comment: https://git.openjdk.org/jdk/pull/15276#issuecomment-1700966823

Reply via email to