On Thu, 30 May 2024 07:33:31 GMT, Abhishek Kumar <abhis...@openjdk.org> wrote:

>> bug6492108.java test always fails in GTK L&F in single as well as dual 
>> screen linux machines. Since this test was not marked as "_headful_" in it's 
>> initial version, it never failed but after the fix of 
>> [JDK-8287051](https://bugs.openjdk.org/browse/JDK-8287051) this test was 
>> problem listed as it always
>> failed which is captured in the JBS.
>> The reason of failure is the pixel color mismatch between JEditorPane and 
>> JTextArea/JTextPane which is caused by the JEditorPane's default opaque 
>> value which is false for GTK3 at 
>> [L767](https://github.com/kumarabhi006/jdk/blob/c642f44bbe1e4cdbc23496a34ddaae30990ce7c0/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKStyle.java#L767).
>> In initial load JEditorPane, JTextArea and JTextPane components are opaque 
>> at 
>> [L718](https://github.com/kumarabhi006/jdk/blame/6c7656678916ff3f5c9fc70efcbb69ce76801458/jdk/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKStyle.java#L718)
>>  but after the fix for 
>> [JDK-8145547](https://bugs.openjdk.org/browse/JDK-8145547) the 
>> implementation was changed to provide conditional support for GTK3 on linux 
>> where few components like Editor Pane, Formatted text Field, Password Field 
>> etc are opaque only if the  [GTK version is not 
>> 3](https://github.com/kumarabhi006/jdk/blame/73d2181d56063f6015e4fc42e130591bee39bc36/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKStyle.java#L746C21-L746C21).
>> JTextPane's issue was observed by 
>> [JDK-8218479](https://bugs.openjdk.org/browse/JDK-8218479) and then the 
>> default opacity is set to true for JTextPane [irrespective of GTK 
>> version](https://github.com/kumarabhi006/jdk/blame/c642f44bbe1e4cdbc23496a34ddaae30990ce7c0/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKStyle.java#L750C16-L750C16).
>> Extending the fix in isOpaque() method in GTKStyle.java for JEditorPane 
>> similar to JTextPane resolves the issue.
>> 
>> Test verified in both single and dual screen linux machines.
>
> Abhishek Kumar has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   background method removed

test/jdk/com/sun/java/swing/plaf/gtk/bug6492108.java line 61:

> 59:                 "com.sun.java.swing.plaf.gtk.GTKLookAndFeel");
> 60:         } catch (Exception e) {
> 61:             System.out.println("GTK LAF is not supported on this system; 
> test passes");

should this be jtreg.testSkipped exception?

test/jdk/com/sun/java/swing/plaf/gtk/bug6492108.java line 70:

> 68:                                      Class<? extends JTextComponent> type)
> 69:         throws Throwable
> 70:     {

I think formatting here looks a little odd, could probably move the bracket 
onto the same line as `throws Throwable` and shift it over to the right to 
match `Class<? extends JTextComponent> type)`

test/jdk/com/sun/java/swing/plaf/gtk/bug6492108.java line 110:

> 108:     }
> 109: 
> 110:     private void onEDT10() {

is this method required? can it be rewritten to not use onEDT10 and 
onBackgroundThread20?

test/jdk/com/sun/java/swing/plaf/gtk/bug6492108.java line 136:

> 134:                 if (refimg.getWidth() != testimg.getWidth() ||
> 135:                     refimg.getHeight() != testimg.getHeight())
> 136:                 {

move bracket to previous line to be consistent

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1621332748
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1621363444
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1621361476
PR Review Comment: https://git.openjdk.org/jdk/pull/19381#discussion_r1621364434

Reply via email to