RFR: 8322135: javax/swing/JTable/JTableScrollPrintTest.java & javax/swing/JTable/PrintAllPagesTest.java throws java.lang.InternalError: HTHEME is null [v4]
Alexey Ivanov
aivanov at openjdk.org
Tue Apr 23 10:30:30 UTC 2024
On Tue, 23 Apr 2024 10:19:40 GMT, Tejesh R <tr at openjdk.org> wrote:
>> Getting a theme for particular dpi failed in windows L&F during print test. Before [JDK-8294427](https://bugs.openjdk.org/browse/JDK-8294427) fix, theme was independent of DPI. After the fix (https://github.com/openjdk/jdk/commit/a63afa4aa62863d1a199a0fb7d2f56ff8fcd04fd) getting a theme in Windows L&F became dependent on DPI. Now it works fine in paint to a frame or window but for printing the DPI is computed with scaling factor which might not be w.r.t set of connected monitors and hence error is returned according to [this from windows document](https://learn.microsoft.com/en-us/windows/win32/api/uxtheme/nf-uxtheme-openthemedatafordpi).
>> The suggested fix is to handle printer cases with default dpi since in error condition 0 is returned with `openThemeImpl(widget, dpi)`. The fix seems to be working fine without causing any regression (Tested in CI systems and also the affected tests).
>
> Tejesh R has updated the pull request incrementally with one additional commit since the last revision:
>
> Review updates
Looks good to me now.
You should add `8322135` to the list of bugs in the tests which failed because of this bug.
Update the copyright year in modified files.
src/java.desktop/windows/classes/sun/awt/windows/ThemeReader.java line 110:
> 108: // See documentation for SetWindowTheme on MSDN.
> 109: setWindowTheme(widget.substring(0, i));
> 110: theme = getOpenThemeValue(widget.substring(i + 2), dpi);
I suggest changing the type of `theme` variable from `Long` to `long` as well as the return value of `openThemeImpl`.
src/java.desktop/windows/classes/sun/awt/windows/ThemeReader.java line 125:
> 123: }
> 124: return theme;
> 125: }
Suggestion:
private static long getOpenThemeValue(String widget, int dpi) {
long theme = openTheme(widget, dpi);
if (theme == 0) {
theme = openTheme(widget, defaultDPI);
}
return theme;
}
Yes, that's what I meant. Let's use the primitive `long`.
-------------
Changes requested by aivanov (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/18706#pullrequestreview-2016847876
PR Review Comment: https://git.openjdk.org/jdk/pull/18706#discussion_r1576019642
PR Review Comment: https://git.openjdk.org/jdk/pull/18706#discussion_r1576017834
More information about the client-libs-dev
mailing list