On Wed, 26 Mar 2025 13:30:48 GMT, Ziad El Midaoui <zelmida...@openjdk.org> 
wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TextFieldSkin.java
>>  line 735:
>> 
>>> 733:         promptNode.fontProperty().bind(getSkinnable().fontProperty());
>>> 734: 
>>> 735:         
>>> promptNode.textProperty().bind(getSkinnable().promptTextProperty().map(s -> 
>>> s.replace("\n", "")));
>> 
>> I recommend to replace all usual newline characters with 
>> `replaceAll("[\r\n]", "")` as I can't think of a compelling reason to remove 
>> only some, but leave others in the string.
>
> The tests show that only LF "\n" is rendered as a new line, there is no need 
> to add more restrictions that is not needed
> and the same was tested by @andy-goryachev-oracle previously in the comments 
> and it confirms the same.

That doesn't sound like a compelling reason to me. In fact, it makes it seems 
like a bug in JavaFX that a line break is only rendered with `\n`, but not with 
`\r\n` or `\r`.

In any case, the goal here is to (semantically) transform a string such that it 
doesn't contain line breaks, and line breaks come in three different usual 
forms. Our goal should always be to do the right thing, and not stop half-way 
and rely on unspecified rendering quirks for the rest.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1716#discussion_r2014250131

Reply via email to