RFR: 8342565: [TestBug] StubTextLayout [v9]

[Andy Goryachev]

On Tue, 25 Mar 2025 18:36:51 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Revert "fixed bad merge"
>>   
>>   This reverts commit 0e9e8ee63895bd1d976398587add5b96958d38aa.
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java line 57:
> 
>> 55:  * Prism TextLayout
>> 56:  */
>> 57: public abstract class PrismTextLayout implements TextLayout {
> 
> Minor: Instead of making this abstract, did you consider leaving it concrete, overriding the methods you need in StubTextLayout? It's fine to leave the change as you have it.

let's see if it works

> modules/javafx.graphics/src/main/java/com/sun/javafx/text/TextRun.java line 45:
> 
>> 43:     int script;
>> 44:     TextSpan span;
>> 45:     com.sun.javafx.scene.text.TextLine line;
> 
> I recommend an import rather than listing the fully-qualified class here and below.

the problem is that we have `com.sun.javafx.text.TextLine implements com.sun.javafx.scene.text.TextLine`

we probably should rename the concrete class to something like `PrismTextLine` for clarity, but that will create a much larger diff.

> modules/javafx.graphics/src/test/java/test/com/sun/javafx/pgstub/StubFontResource.java line 99:
> 
>> 97:         if (bold == null) {
>> 98:             String name = font.getStyle();
>> 99:             bold = name.toLowerCase(Locale.US).contains("bold");
> 
> Should this be `Locale.ROOT` or does it really need to be `Locale.US`?

Locale.ROOT is probably a better choice.  the main reason is to avoid unexpected conversion in non-English locales.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1667#discussion_r2012989686
PR Review Comment: https://git.openjdk.org/jfx/pull/1667#discussion_r2012985556
PR Review Comment: https://git.openjdk.org/jfx/pull/1667#discussion_r2012987712