On Mon, 12 Feb 2024 10:18:14 GMT, Prasanta Sadhukhan <[email protected]> 
wrote:

>> Existing regression test is failing because textfield height is not as per 
>> test's expectation..Seems like the indic character being tried to render is 
>> not being loaded (probably because of missing glyphs) leading to 0 
>> preferredSpan from 
>> [BoxView](https://github.com/openjdk/jdk/blame/5a74c2a67ebcb47e51732f03c4be694bdf920469/src/java.desktop/share/classes/javax/swing/text/BoxView.java#L545-L552)
>>  which is superclass for `i18nFieldVIew`, the field view for Indic 
>> characters.
>> Tried block character in the test which is now being loaded properly leading 
>> to correct height..
>
> Prasanta Sadhukhan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Check with FOnt.canDisplay

test/jdk/javax/swing/plaf/basic/BasicTextUI/8001470/bug8001470.java line 44:

> 42:     private static JTextField textField1;
> 43:     private static JTextField textField2;
> 44:     private static boolean fontFound = false;

Suggestion:

    private static volatile boolean fontFound = false;

The value is set on EDT, but the value is read on the main thread.

test/jdk/javax/swing/plaf/basic/BasicTextUI/8001470/bug8001470.java line 58:

> 56:                     Font[] allFonts = ge.getAllFonts();
> 57:                     for (int x = 0; x < allFonts.length; x++) {
> 58:                         if ((allFonts[x].canDisplay('\u0e01')) && 
> (allFonts[x].canDisplay('\u0c01'))) {

You may want to declare the characters as constants so that you create text 
fields with the characters you're searching the font for.

test/jdk/javax/swing/plaf/basic/BasicTextUI/8001470/bug8001470.java line 72:

> 70: 
> 71:                     textField1 = new JTextField("\u0e01");
> 72:                     textField2 = new JTextField("\u0c01");

~~Wouldn't it be enough to call `getPreferredSize()`?~~

No; if you call `getPreferredSize()`, the bug 
[JDK-8001470](https://bugs.openjdk.org/browse/JDK-8001470) can't be reproduced.

test/jdk/javax/swing/plaf/basic/BasicTextUI/8001470/bug8001470.java line 80:

> 78:                 }
> 79:             });
> 80:             Thread.sleep(1000);

You can skip waiting `if (!fontFound)`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17528#discussion_r1486735416
PR Review Comment: https://git.openjdk.org/jdk/pull/17528#discussion_r1486726564
PR Review Comment: https://git.openjdk.org/jdk/pull/17528#discussion_r1486728384
PR Review Comment: https://git.openjdk.org/jdk/pull/17528#discussion_r1486732123

Reply via email to