8314215: Trailing Spaces before Line Breaks Affect the Center Alignment of Text #1236

hjohn · 2023-09-10T20:50:18Z

There are a number of tickets open related to text rendering:

https://bugs.openjdk.org/browse/JDK-8314215

https://bugs.openjdk.org/browse/JDK-8145496

https://bugs.openjdk.org/browse/JDK-8129014

They have in common that wrapped text is taking the trailing spaces on each wrapped line into account when calculating where to wrap. This looks okay for text that is left aligned (as the spaces will be trailing the lines and generally aren't a problem, but looks weird with CENTER and RIGHT alignments. Even with LEFT alignment there are artifacts of this behavior, where a line like AAA BBB CCC (note the double spaces) gets split up into AAA , BBB and CCC, but if space reduces further, it will wrap too early because the space is taken into account (ie. AAA may still have fit just fine, but AAA doesn't, so the engine wraps it to AA + A or something).

The fix for this is two fold; first the individual lines of text should not include any trailing spaces into their widths; second, the code that is taking the trailing space into account when wrapping should ignore all trailing spaces (currently it is ignoring all but one trailing space). With these two fixes, the layout in LEFT/CENTER/RIGHT alignments all look great, and there is no more early wrapping due to a space being taking into account while the actual text still would have fit (this is annoying in tight layouts, where a line can be wrapped early even though it looks like it would have fit).

If it were that simple, we'd be done, but there may be another issue here that needs solving: wrapped aligned TextArea's.

TextArea don't directly support text alignment (via a setTextAlignment method like Label) but you can change it via CSS.

For Left alignment + wrapping, TextArea will ignore any spaces typed before a line that was wrapped. In other words, you can type spaces as much as you want, and they won't show up and the cursor won't move. The spaces are all getting appended to the previous line. When you cursor through these spaces, the cursor can be rendered out of the control's bounds. To illustrate, if you have the text AAA BBB CCC, and the text gets wrapped to AAA, BBB, CCC, typing spaces before BBB will not show up. If you cursor back, the cursor may be outside the control bounds because so many spaces are trailing AAA.

The above behavior has NOT changed, is pretty standard for wrapped text controls, and IMHO does not need further attention.

Now, for RIGHT alignment, the new code does change things a bit. Where before the single trailing space that was not collapsed was shown at the end of each wrapped line, this space is now gone. Typing further spaces after AAA will not show up, but they are still there, and moving the cursor through them will move the cursor beyond the right side of the control's bounds (just like LEFT alignment has such a case). Some Text rendering implementations will "clamp" the cursor to prevent this (browsers), while others will render the cursor in the margin (but eventually the cursor disappears if there is not enough margin and there are too many spaces).

Personally, I think this is absolutely fine. Wrapped text controls all have this kind of behavior, where typing a space just before the wrapping point seemingly has no effect until a non-space character is typed.

For CENTER alignment, the case is similar to LEFT alignment, except there is a bit less space available to render "invisible" spaces.

Screenshots

Using Labels containing the text AAA BBB CCC (note the doubled space between words)

First wrapping point

Before

Note the odd trailing space in Center and Right alignments.

After

Small but text would still fit

Before

Note how the wrapping occurs earlier than needed, leading to "empty" lines that contained invisible spaces

After

Note how everything still fits

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

JDK-8314215: Trailing Spaces before Line Breaks Affect the Center Alignment of Text (Bug - P4)

Reviewers

Karthik P K (@karthikpandelu - Committer)
Andy Goryachev (@andy-goryachev-oracle - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1236/head:pull/1236
$ git checkout pull/1236

Update a local copy of the PR:
$ git checkout pull/1236
$ git pull https://git.openjdk.org/jfx.git pull/1236/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1236

View PR using the GUI difftool:
$ git pr show -t 1236

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1236.diff

Webrev

Link to Webrev Comment

bridgekeeper · 2023-09-10T20:50:45Z

👋 Welcome back jhendrikx! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

mlbridge · 2023-09-10T21:14:06Z

Webrevs

hjohn · 2023-09-10T21:23:53Z

@prrace I've done an attempt to make a fix for this behavior, if you could take a look and let me know what you think, that'd be great

andy-goryachev-oracle · 2023-09-14T22:41:20Z

modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java

+        int trailingSpaces = 0;
+
+        for (int i = length + startOffset - 1; i >= startOffset; i--) {
+            if (chars[i] != ' ') {


should Character.isWhitespace() be used instead (think of symbols like U+2001 that might break, see https://en.wikipedia.org/wiki/Whitespace_character)

I'm not entirely sure, perhaps Phil Race @prrace could answer that? There are loops that just check for 0x20, but also more complicated loops that use Character.isWhitespace.

I think this logic needs to support all whitespace characters that might break. Since there is no method in Character to indicate the breaking behavior, I wonder if we ought to either request one, or simply hardcode a list here.

hjohn · 2023-09-23T14:38:15Z

@kevinrushforth I think this PR may be worth consideration, although I am not 100% sure that I haven't missed anything, the examples look a lot better with this fix.

hjohn · 2023-09-23T14:39:30Z

Here's the program I used to make the screenshots:

import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.control.Label;
import javafx.scene.layout.GridPane;
import javafx.scene.text.Font;
import javafx.scene.text.TextAlignment;
import javafx.stage.Stage;

public class TextFlowProblem2 {
  public static void main(String[] args) {
    Application.launch(App.class);
  }

  public static class App extends Application {

     /**
      * @param args the command line arguments
      */
     public static void start(String[] args) {
       Application.launch(args);
     }

    @Override
    public void start(Stage stage) {
      Label label1 = new Label("AAA  BBB  CCC");

      label1.setFont(Font.font("courier new", 60));
      label1.setWrapText(true);
      label1.setTextAlignment(TextAlignment.LEFT);
      label1.setStyle("-fx-border-color: red;");

      Label label2 = new Label("AAA  BBB  CCC");

      label2.setFont(Font.font("courier new", 60));
      label2.setWrapText(true);
      label2.setTextAlignment(TextAlignment.CENTER);
      label2.setStyle("-fx-border-color: red;");

      Label label3 = new Label("AAA  BBB  CCC");

      label3.setFont(Font.font("courier new", 60));
      label3.setWrapText(true);
      label3.setTextAlignment(TextAlignment.RIGHT);
      label3.setStyle("-fx-border-color: red;");

      GridPane gridPane = new GridPane();

      Label alignmentLabel1 = new Label("Left Aligned");
      Label alignmentLabel2 = new Label("Center Aligned");
      Label alignmentLabel3 = new Label("Right Aligned");

      alignmentLabel1.setMinWidth(100);
      alignmentLabel2.setMinWidth(100);
      alignmentLabel3.setMinWidth(100);

      gridPane.addRow(0, alignmentLabel1, label1);
      gridPane.addRow(1, alignmentLabel2, label2);
      gridPane.addRow(2, alignmentLabel3, label3);

      stage.setScene(new Scene(gridPane));

      stage.setWidth(1024);
      stage.setHeight(1024);
      stage.show();
    }
  }
}

kevinrushforth · 2023-09-30T17:36:32Z

@prrace Can you take a look?

kevinrushforth · 2023-10-27T16:18:14Z

/reviewers 2

openjdk · 2023-10-27T16:19:57Z

@kevinrushforth
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

mstr2 · 2023-10-30T05:34:54Z

I've tested AAA BBB CCC in Chrome, and the text is rendered pretty much exactly as your "before" version:

Center:

Right:

Here's the HTML:

<p style="white-space:pre-wrap; font:90pt courier new; text-align:right">AAA  BBB  CCC</p>

Edit: I just discovered that there are more white-space settings than I thought. Using white-space:pre-line the rendered text looks pretty much exactly like your "after" version. Maybe we also need this toggle in JavaFX?

hjohn · 2023-10-30T08:52:27Z

I've tested AAA BBB CCC in Chrome, and the text is rendered pretty much exactly as your "before" version:

For the current rendering that JavaFX does, there is no browser equivalent, although pre-wrap comes close, JavaFX will take the spaces into account when adding the ellipsis (in HTML can test with overflow: hidden; text-overflow: ellipsis;) and displays odd empty lines in between the cut-off lines (see the 2nd before picture). I suppose this could also be considered a separate bug.

For browsers, pre-line is the normal equivalent that preserves spaces; this probably should be the default setting for FX as spaces are considered significant in FX controls. This what this PR provides.

Although I definitely think the pre-line (or normal but preserving white space) rendering should have been standard from the beginning, this could be provided with a CSS style (but that will need a default as well, and the current one isn't ideal as default IMHO). I'm however not sure if that's really worth it as JavaFX labels and text fields are quite different from browser elements. I'm unaware of other UI toolkits offering such settings.

nlisker · 2023-10-30T12:12:23Z

I tested the behavior of MS Word. The text is AAA BBB CCC.

Left aligned:

Center aligned:

Right aligned:

andy-goryachev-oracle · 2023-10-30T19:00:08Z

From https://www.unicode.org/reports/tr14-4/

5.6 Break opportunity after characters (A)
Breaking Spaces
SPACE (SP) � U+0020

The space characters are explicit break opportunities, but spaces at the end of a line are not measured for fit. If there is a sequence of space characters, and breaking after any of the space characters would result in the same visible line, the line breaking position after the last space character in the sequence is the locally most optimal one. In other words, since the last character measured for fit is BEFORE the space character, any number of space characters are kept together invisibly on the previous line and the first non-space character starts the next line.

It is sometimes convenient to use SP, but not the other breaking spaces to override context based behavior of other characters under the "anywhere, except where prohibited" style of line breaking (context analysis style 2).

EN QUAD � U+2000
EM QUAD � U+2001
EN SPACE � U+2002
EM SPACE � U+2003
THREE-PER-EM SPACE � U+2004
FOUR-PER-EM SPACE � U+2005
SIX-PER-EM SPACE � U+2006
PUNCTUATION SPACE � U+2008
THIN SPACE � U+2009
HAIR SPACE � U+200A

The preceding list of characters all have a specific width, but behave otherwise as breaking spaces .

ZERO WIDTH SPACE (ZWSP) � U+200B

This character does not have width. It is used in a style 2 context analysis to provide additional (invisible) break opportunities.

IDEOGRAPHIC SPACE � U+3000

This character has the width of an ideograph but like ZWSP is fully subject to the style 2 context analysis.

A quick check with (the latest?) MS Word 2208 on Windows shows that, at least with EN QUAD U+2000 it is treated as a regular character (i.e. it is always "displayed" even if right aligned).

mstr2 · 2023-10-30T19:09:17Z

I've tested AAA BBB CCC in Chrome, and the text is rendered pretty much exactly as your "before" version:

For the current rendering that JavaFX does, there is no browser equivalent, although pre-wrap comes close, JavaFX will take the spaces into account when adding the ellipsis (in HTML can test with overflow: hidden; text-overflow: ellipsis;) and displays odd empty lines in between the cut-off lines (see the 2nd before picture). I suppose this could also be considered a separate bug.

For browsers, pre-line is the normal equivalent that preserves spaces; this probably should be the default setting for FX as spaces are considered significant in FX controls. This what this PR provides.

Although I definitely think the pre-line (or normal but preserving white space) rendering should have been standard from the beginning, this could be provided with a CSS style (but that will need a default as well, and the current one isn't ideal as default IMHO). I'm however not sure if that's really worth it as JavaFX labels and text fields are quite different from browser elements. I'm unaware of other UI toolkits offering such settings.

It seems there's no browser equivalent to your proposed behavior, either. pre-line does not preserve spaces, but your proposal does; it merely doesn't preserve end-of-line spaces.

While that might be a sensible default behavior, I'm not sure about the "invisible input" behavior that you'd get if you entered spaces at the end of a line.

hjohn · 2023-10-30T19:53:50Z

It seems there's no browser equivalent to your proposed behavior, either. pre-line does not preserve spaces, but your proposal does; it merely doesn't preserve end-of-line spaces.

Yeah, I missed that somehow. pre-line looks close but collapsed inter word spaces; pre-wrap doesn't, but its wrapping behavior is different. I get the impression that pre-wrap is the one you want. It's described as "Whitespace is preserved by the browser. Text will wrap when necessary, and on line breaks", which sounds correct, apart from it also preserving some space when reflowing.

Perhaps Chrome is not doing it quite right? I tested Firefox (also Opera and some others but most of those are Chromium based), and it looks like browsers are disagreeing; Firefox is doing it differently (with pre-wrap):

Edit: Looks like Firefox will try to wrap too early though (it adds ellipsis too soon) so browsers don't seem to be a really authoritive source for this behavior.

While that might be a sensible default behavior, I'm not sure about the "invisible input" behavior that you'd get if you entered spaces at the end of a line.

That's done almost everywhere, just try it in the comment box here on Github for example. Type sufficient text to reach the edge of the box, then type a lot of spaces. They seem to "disappear", and they don't appear when the text reflows. If you then cursor back, the cursor won't appear until you passed all the spaces you typed; or if you break the line in some other place, all those spaces are still there. It feels odd, but I think it is not a bad way to handle this kind of thing.

The example of MS Word that Nir showed also shows that the extra spaces just go past the margin. Photoshop does the same (it's in one the tickets).

hjohn · 2023-10-30T19:56:50Z

@andy-goryachev-oracle

The space characters are explicit break opportunities, but spaces at the end of a line are not measured for fit. If there is a sequence of space characters, and breaking after any of the space characters would result in the same visible line, the line breaking position after the last space character in the sequence is the locally most optimal one. In other words, since the last character measured for fit is BEFORE the space character, any number of space characters are kept together invisibly on the previous line and the first non-space character starts the next line.

That's certainly a good description of how the breaking should be handled.

bridgekeeper · 2023-11-28T03:52:17Z

@hjohn This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

hjohn · 2023-11-28T09:30:45Z

This really deserves to be fixed. Any progress on getting this reviewed?

andy-goryachev-oracle · 2023-11-28T16:50:03Z

Do we want to limit this feature to ASCII spaces explicitly?

bridgekeeper · 2023-12-26T17:48:18Z

@hjohn This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

hjohn · 2023-12-26T18:31:26Z

keep open

bridgekeeper · 2024-01-24T01:06:09Z

@hjohn This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

andy-goryachev-oracle · 2024-01-25T16:18:37Z

Regarding other whitespace characters.
Using \u2003 em space, pasting the following text

AAA    BBB    CCC

the results as seen with MS Word 16.78 (23100802) on macOS 14.2.1 indicate that it treats em space as whitespace:

andy-goryachev-oracle · 2024-01-25T16:33:57Z

Even if we forget other whitespace characters and use 0x20 spaces, there is a weird problem where "CCC" jumps when changing the width of the window:

I am testing this PR branch merged with the latest master and a slightly different test program:

    @Override
    public void start(Stage stage) {
        String text =
            //"AAA\u2003\u2003\u2003\u2003BBB\u2003\u2003\u2003\u2003CCC\u2003\u2003\u2003\u2003";
            "AAA     BBB      CCC      ";

        TextFlow t = new TextFlow(new Text(text));
        t.setStyle("-fx-font-size:200%;");
        t.setTextAlignment(TextAlignment.CENTER);
        
        BorderPane root = new BorderPane(t);
        Scene scene = new Scene(root, 595, 150, Color.BEIGE);
        
        stage.setTitle("Text Alignment");
        stage.setScene(scene);
        stage.show();
    }

openjdk · 2024-04-10T21:52:59Z

@hjohn this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout feature/wrapped-aligned-text-rendering
git fetch https://git.openjdk.org/jfx.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

xulihang · 2024-04-13T08:05:23Z

Hi there, could this fix be merged? As I am creating a typesetting tool, this is very important for it. xulihang/ImageTrans-docs#482

andy-goryachev-oracle · 2024-04-22T16:52:16Z

This PR is very close to be ready. It looks like there are only two things that block it:

merge conflict
resolve the problems with the fonts (could we pick the font(s) that are available via some utility method?)

…pped-aligned-text-rendering

hjohn · 2024-04-23T05:30:44Z

@karthikpandelu @kevinrushforth @andy-goryachev-oracle

I split the failing Mac tests into Windows and Mac variants. However, the Mac specific tests probably still fail. If one of you could post the test errors, I can fix the values used as I don't have a Mac to test on.

hjohn · 2024-04-23T05:50:45Z

resolve the problems with the fonts (could we pick the font(s) that are available via some utility method?)

I already left a comment on this before, see here: #1236 (comment)

The TextLayoutTest tests the reflow logic in PrismTextLayout. The code under test is not platform or font specific, it doesn't have different branches for different fonts or platforms. Adding more fonts for specific platforms therefore would exercise the exact same code paths twice without a tangible benefit. If the tests fail for Tahoma, the same tests would fail for Monaco. If it fails on one platform, the exact same failures would occur on another platform.

Duplicating the tests for a new font would require running the tests, checking all the exact metric values, and copying those to the duplicated test. Adding new tests becomes more of a maintenance burden as now two tests need to be maintained and updated (both would fail if one fails).

Ideally, these tests should be real Unit tests, with a Font definition stored in src/test/resources and without requiring the FX platform to run -- the tested code is just some maths after all. However, FX is insufficiently abstracted to allow for loading certain classes without the platform running, and storing a specific font in the repository may not be possible due to licensing issues (even just to check IF there are licensing issues).

karthikpandelu · 2024-04-23T12:40:57Z

@karthikpandelu @kevinrushforth @andy-goryachev-oracle

I split the failing Mac tests into Windows and Mac variants. However, the Mac specific tests probably still fail. If one of you could post the test errors, I can fix the values used as I don't have a Mac to test on.

Here is the result of TextLayoutTest in MacOS M1 system.

TextLayoutTest > caseTest(Case) > [12] SOFT_WRAP_WITH_COMPLEX_TEXT_ON_MAC FAILED
    org.opentest4j.AssertionFailedError: left aligned rich text (spans): line 1 for SOFT_WRAP_WITH_COMPLEX_TEXT_ON_MAC ==> expected: <RectBounds { minX:0.0, minY:-10.863281, maxX:93.134766, maxY:2.9355469} (w:93.134766, h:13.798828)> but was: <RectBounds { minX:0.0, minY:-10.863281, maxX:91.04719, maxY:2.9355469} (w:91.04719, h:13.798828)>
        at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
        at app//org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
        at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1152)
        at app//test.com.sun.javafx.text.TextLayoutTest.caseTest(TextLayoutTest.java:533)

TextLayoutTest > caseTest(Case) > [13] SOFT_WRAP_WITH_COMPLEX_TEXT_AND_EXTRA_SPACE_ON_MAC FAILED
    org.opentest4j.AssertionFailedError: left aligned rich text (spans): line 1 for SOFT_WRAP_WITH_COMPLEX_TEXT_AND_EXTRA_SPACE_ON_MAC ==> expected: <RectBounds { minX:0.0, minY:-10.863281, maxX:93.134766, maxY:2.9355469} (w:93.134766, h:13.798828)> but was: <RectBounds { minX:0.0, minY:-10.863281, maxX:91.04719, maxY:2.9355469} (w:91.04719, h:13.798828)>
        at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
        at app//org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
        at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1152)
        at app//test.com.sun.javafx.text.TextLayoutTest.caseTest(TextLayoutTest.java:533)

Following are the skipped tests:

TextLayoutTest > caseTest(Case) > [10] SOFT_WRAP_WITH_COMPLEX_TEXT_ON_WINDOWS SKIPPED

TextLayoutTest > caseTest(Case) > [11] SOFT_WRAP_WITH_COMPLEX_TEXT_AND_EXTRA_SPACE_ON_WINDOWS SKIPPED

TextLayoutTest > fixedComplexTestsToEnsureNoFurtherRegressions() SKIPPED

TextLayoutTest > complexTestsThatAreBrokenSince2013() SKIPPED

hjohn · 2024-04-23T13:39:00Z

Here is the result of TextLayoutTest in MacOS M1 system.

Thanks very much, I've updated the values now, so no tests should fail anymore on Mac.

Following are the skipped tests:

TextLayoutTest > caseTest(Case) > [10] SOFT_WRAP_WITH_COMPLEX_TEXT_ON_WINDOWS SKIPPED

TextLayoutTest > caseTest(Case) > [11] SOFT_WRAP_WITH_COMPLEX_TEXT_AND_EXTRA_SPACE_ON_WINDOWS SKIPPED

TextLayoutTest > fixedComplexTestsToEnsureNoFurtherRegressions() SKIPPED

TextLayoutTest > complexTestsThatAreBrokenSince2013() SKIPPED

That looks as expected.

tests/system/src/test/java/test/com/sun/javafx/text/TextLayoutTest.java

andy-goryachev-oracle · 2024-04-23T17:36:20Z

tests/system/src/test/java/test/com/sun/javafx/text/TextLayoutTest.java

@@ -0,0 +1,521 @@
+/*
+ * Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved.


since it's been modified, please change the year

tests/system/src/test/java/test/com/sun/javafx/text/TextLayoutTest.java

hjohn · 2024-04-23T19:21:43Z

since it's been modified, please change the year

Sorry, I stopped doing that, I thought this was automated.

andy-goryachev-oracle

LGTM!

karthikpandelu

LGTM

kevinrushforth · 2024-04-24T15:23:39Z

One meta comment (not directly related to this review).

since it's been modified, please change the year

Sorry, I stopped doing that, I thought this was automated.

It is optional to update copyrights in a PR for a modified file (new files do need a correctly formatted copyright file with the right year). Ambarish periodically runs a script to update them. If you do update copyrights, you need to make sure that the year is right (if, for example, the PR was started in one year and finished the next yet). I tend to not update them for PRs that I expect to backport, since it increases the likelihood of not being able to use the /backport command.

hjohn · 2024-04-26T10:22:42Z

@kevinrushforth This is ready to integrate, but I know the headful tests don't run automatically. Would it be wise to run these once more before integrating?

Also, I re-read the issues https://bugs.openjdk.org/browse/JDK-8145496 and https://bugs.openjdk.org/browse/JDK-8129014, and I'm positive they're duplicates that would be fixed by this solution. Shall I add these to this PR?

kevinrushforth · 2024-04-26T15:59:49Z

@kevinrushforth This is ready to integrate, but I know the headful tests don't run automatically. Would it be wise to run these once more before integrating?

Yes, that would be a good idea. I'll do that and report results here.

Also, I re-read the issues https://bugs.openjdk.org/browse/JDK-8145496 and https://bugs.openjdk.org/browse/JDK-8129014, and I'm positive they're duplicates that would be fixed by this solution.

I think it's better to close JDK-8145496 as a duplicate. As for JDK-8129014, it is already closed (as not an issue), so we could either leave it alone or reopen it and reclose it as a Duplicate.

kevinrushforth · 2024-04-26T19:36:20Z

The newly added test passes on all platforms.

@hjohn I assigned JDK-8145496 to you to verify that it is fixed by this PR. If so, please close it as a duplicate of this bug. Since JDK-8129014 is an old (JDK 7) issue, let's just leave it alone.

hjohn · 2024-04-28T18:21:20Z

/integrate

openjdk · 2024-04-28T18:22:25Z

Going to push as commit 398f104.
Since your change was applied there have been 4 commits pushed to the master branch:

c23ac74: 8273657: TextField: all text content must be selected initially
d8ca38a: 8146918: ConcurrentModificationException in MediaPlayer
e72d681: 8330701: Fix Eclipse project in tests/manual/text
3333740: 8328577: Toolbar's overflow button overlaps the items

Your commit was automatically rebased without conflicts.

openjdk · 2024-04-28T18:22:32Z

@hjohn Pushed as commit 398f104.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Fix to ignore trailing white space for wrapping and alignment

bcca81e

hjohn changed the title ~~Fix to ignore trailing white space for wrapping and alignment~~ JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text Sep 10, 2023

hjohn marked this pull request as ready for review September 10, 2023 21:09

openjdk bot added the rfr Ready for review label Sep 10, 2023

andy-goryachev-oracle reviewed Sep 14, 2023

View reviewed changes

kevinrushforth requested a review from prrace September 30, 2023 17:36

xulihang mentioned this pull request Nov 5, 2023

Spaces interfering with center alignment xulihang/ImageTrans-docs#482

Open

openjdk bot added the merge-conflict Pull request has merge conflict with target branch label Apr 10, 2024

hjohn added 3 commits April 23, 2024 06:31

Merge branch 'master' of https://git.openjdk.org/jfx into feature/wra…

628805b

…pped-aligned-text-rendering

Fix typo

ba1ce8f

Make some tests conditional on platform

c3ca720

openjdk bot removed the merge-conflict Pull request has merge conflict with target branch label Apr 23, 2024

Fix mac test values

6b8f18d

andy-goryachev-oracle reviewed Apr 23, 2024

View reviewed changes

Update copyright years

9bdd8f8

andy-goryachev-oracle approved these changes Apr 23, 2024

View reviewed changes

karthikpandelu approved these changes Apr 24, 2024

View reviewed changes

openjdk bot added the ready Ready to be integrated label Apr 24, 2024

openjdk bot added the integrated Pull request has been integrated label Apr 28, 2024

openjdk bot closed this Apr 28, 2024

openjdk bot removed ready Ready to be integrated rfr Ready for review labels Apr 28, 2024

		@@ -0,0 +1,521 @@
		/*
		* Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved.

8314215: Trailing Spaces before Line Breaks Affect the Center Alignment of Text #1236

8314215: Trailing Spaces before Line Breaks Affect the Center Alignment of Text #1236

Conversation

hjohn commented Sep 10, 2023 • edited by openjdk bot

Screenshots

First wrapping point

Before

After

Small but text would still fit

Before

After

Progress

Issue

Reviewers

Reviewing

Webrev

bridgekeeper bot commented Sep 10, 2023

mlbridge bot commented Sep 10, 2023 • edited

Webrevs

hjohn commented Sep 10, 2023

andy-goryachev-oracle Sep 14, 2023 • edited

Choose a reason for hiding this comment

hjohn Sep 23, 2023

Choose a reason for hiding this comment

andy-goryachev-oracle Sep 25, 2023

Choose a reason for hiding this comment

hjohn commented Sep 23, 2023

hjohn commented Sep 23, 2023

kevinrushforth commented Sep 30, 2023

kevinrushforth commented Oct 27, 2023

openjdk bot commented Oct 27, 2023

mstr2 commented Oct 30, 2023 • edited

hjohn commented Oct 30, 2023

nlisker commented Oct 30, 2023

andy-goryachev-oracle commented Oct 30, 2023

mstr2 commented Oct 30, 2023

hjohn commented Oct 30, 2023 • edited

hjohn commented Oct 30, 2023

bridgekeeper bot commented Nov 28, 2023

hjohn commented Nov 28, 2023

andy-goryachev-oracle commented Nov 28, 2023

bridgekeeper bot commented Dec 26, 2023

hjohn commented Dec 26, 2023

bridgekeeper bot commented Jan 24, 2024

andy-goryachev-oracle commented Jan 25, 2024

andy-goryachev-oracle commented Jan 25, 2024

openjdk bot commented Apr 10, 2024

xulihang commented Apr 13, 2024 • edited by bridgekeeper bot

andy-goryachev-oracle commented Apr 22, 2024

hjohn commented Apr 23, 2024

hjohn commented Apr 23, 2024 • edited

karthikpandelu commented Apr 23, 2024

hjohn commented Apr 23, 2024

andy-goryachev-oracle Apr 23, 2024

Choose a reason for hiding this comment

hjohn commented Apr 23, 2024

andy-goryachev-oracle left a comment

Choose a reason for hiding this comment

karthikpandelu left a comment

Choose a reason for hiding this comment

kevinrushforth commented Apr 24, 2024

hjohn commented Apr 26, 2024 • edited

kevinrushforth commented Apr 26, 2024

kevinrushforth commented Apr 26, 2024

hjohn commented Apr 28, 2024

openjdk bot commented Apr 28, 2024

openjdk bot commented Apr 28, 2024

hjohn commented Sep 10, 2023 •

edited by openjdk bot

mlbridge bot commented Sep 10, 2023 •

edited

andy-goryachev-oracle Sep 14, 2023 •

edited

mstr2 commented Oct 30, 2023 •

edited

hjohn commented Oct 30, 2023 •

edited

xulihang commented Apr 13, 2024 •

edited by bridgekeeper bot

hjohn commented Apr 23, 2024 •

edited

hjohn commented Apr 26, 2024 •

edited