On Wed, 16 Aug 2023 16:53:39 GMT, Alexey Ivanov <[email protected]> wrote:
>> Harshitha Onkar has updated the pull request incrementally with one >> additional commit since the last revision: >> >> removed robot.waitForIdle() calls > > src/java.desktop/windows/native/libawt/windows/awt_MenuItem.cpp line 708: > >> 706: | MIIM_STATE | MIIM_SUBMENU | MIIM_TYPE; >> 707: >> 708: ::GetMenuItemInfo(hMenu, GetID(), FALSE, &mii); > > Should we update the `fMask` to use `MIIM_FTYPE` instead of `MIIM_TYPE`? The > latter requests both `fType` and `dwTypeData` whereas the former requests > only `fType`, as we don't use `dwTypeData`. > > (I understand that this code was written before Windows introduced new values > for `fMask` field, which probably happened in Windows 98. Now we can update > the code to document our intentions clearer.) Updated fMask to use MIIM_FTYPE > test/jdk/java/awt/MenuItem/SetLabelTest.java line 161: > >> 159: } >> 160: return passed; >> 161: } > > My only concern here is that the test doesn't ensure that the menu has > updated on the screen. > > The data inside the Java object are always updated? > > The test should probably make a screenshot before a label is changed, then > make a screenshot after the label is changed — there should be differences on > the screenshots. Test updated to compare after and before screenshots. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15276#discussion_r1309091859 PR Review Comment: https://git.openjdk.org/jdk/pull/15276#discussion_r1309089812
