On Mon, 5 Jun 2023 11:16:52 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion in
On Tue, 6 Jun 2023 20:14:41 GMT, Andy Goryachev wrote:
>> This case is same as the one which we saw with red heart as the emoji
>> sequence mentioned here is also a U+FE0F Variation Selector-16 character.
>> Since it exhibits different behavior in different platforms we decided to
>> remove it
On Tue, 6 Jun 2023 09:46:53 GMT, Karthik P K wrote:
>> tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePairInsertionIndexTest.java
>> line 151:
>>
>>> 149: private void addMultipleEmojis() {
>>> 150: Util.runAndWait(() -> {
>>> 151: text = new Text("😊
On Mon, 5 Jun 2023 11:16:52 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion in
On Mon, 5 Jun 2023 15:46:00 GMT, Andy Goryachev wrote:
>> Karthik P K has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add out of bound check for insertion index
>
> tests/system/src/test/java/test/robot/javafx/scene/TextFlowSurrogatePair
On Mon, 5 Jun 2023 17:12:41 GMT, Andy Goryachev wrote:
>> Karthik P K has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add out of bound check for insertion index
>
> FYI, I've added "Enter Text" button to Text/TextFlow pages in the Monkey
On Mon, 5 Jun 2023 11:16:52 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion in
On Mon, 5 Jun 2023 11:16:52 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion in
On Fri, 2 Jun 2023 20:57:06 GMT, Phil Race wrote:
>> you are right, it cannot be explained by a null text.
>>
>> so if i understand you correctly, the question is about validity of
>> `insertionIndex += 1` on lines 463 and 469.
>>
>> 463: should it be set to text.length() instead?
>>
>> 469:
> Since surrogate pairs are internally considered as 2 characters and text
> field is null in `HitInfo` when `getInsertionIndex` is invoked from
> `TextFlow`, wrong insertion index was returned.
>
> Updated code to calculate insertion index in `getHitInfo` method of
> `PrismTextLayout` class wh
On Fri, 2 Jun 2023 19:41:04 GMT, Andy Goryachev wrote:
>> No, it wasn't due to null text. That is provably impossible.
>>
>> The exception from JDK-8302511 is as follows (copied from the bug report).
>>
>> Exception in thread "JavaFX Application Thread"
>> java.lang.IllegalArgumentException: o
On Fri, 2 Jun 2023 19:18:13 GMT, Phil Race wrote:
>> The exception in 8302511 was likely caused by a prior exception due to null
>> text, which corrupted insertionIndex value in HitInfo.
>>
>> The goal of this change is to always compute insertionIndex (an I believe we
>> do it correctly this
On Fri, 2 Jun 2023 18:50:07 GMT, Andy Goryachev wrote:
>> I did note that TextFlow passed null which really looked like a WIP to me
>> but its a separate issue.
>> The exception in 8302511 which is the entire subject of that bug isn't
>> possible if text == null.
>> My point is that if you pass
On Fri, 2 Jun 2023 18:37:19 GMT, Phil Race wrote:
>>> Assuming line 473 is the one still there today, it looks to me as if that
>>> would be reached if you had the caret on an empty line that isn't the last
>>> line.
>>
>> In this case also code hits the condition in 451 as the text run is not
On Fri, 2 Jun 2023 07:19:01 GMT, Karthik P K wrote:
>> The main issue is that insertion index was not computed in the
>> PrismTextLayout, supposedly to allow for delaying the computation until it's
>> needed (if needed at all). A good intention, but the execution had two
>> issues:
>> 1. HitI
On Thu, 1 Jun 2023 22:54:06 GMT, Andy Goryachev wrote:
>> Regarding
>> "If we can initialize insertionIndex to a positive value, then the buggy
>> code in HitInfo.getInsertionIndex()
>> will never get executed, and we can remove it later (in JDK-8302511)."
>>
>> it seems to me that this new cod
On Thu, 1 Jun 2023 20:55:33 GMT, Phil Race wrote:
>> Assuming line 473 is the one still there today, it looks to me as if that
>> would be reached if you had the caret on an empty line that isn't the last
>> line.
>
> Regarding
> "If we can initialize insertionIndex to a positive value, then th
On Thu, 1 Jun 2023 20:53:07 GMT, Phil Race wrote:
>>> I couldn't create the scenario to hit line 473. If you have any suggestions
>>> please let me know.
>>
>> I can't seem to trigger this code path (I wonder if it's possible to hit it
>> at all). Anyway, the code there looks correct to me, e
On Tue, 16 May 2023 17:53:00 GMT, Andy Goryachev wrote:
>> Initializing the insertion index directly with character index looks better.
>> In this way we can avoid additional conditional logic as well. Changed code
>> to do so.
>> Added additional test case. This tests the condition present in
On Mon, 22 May 2023 16:30:40 GMT, Kevin Rushforth wrote:
> Can you file a new (P4) testbug as a follow-up issue to use it replace the
> two existing uses of `firePulse`?
Created [JDK-8308608](https://bugs.openjdk.org/browse/JDK-8308608)
-
PR Comment: https://git.openjdk.org/jfx/pu
On Mon, 22 May 2023 17:14:30 GMT, Andy Goryachev wrote:
> what do you people think of the number of pulses parameter? is 10 enough? is
> there a reasonable upper limit that's needed for any processing to settle?
I think 10 should be a reasonable number of pulses for our tests. Most graphs
sett
On Mon, 22 May 2023 14:21:33 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Mon, 22 May 2023 14:21:33 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Mon, 22 May 2023 14:21:33 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Fri, 19 May 2023 16:24:44 GMT, Andy Goryachev wrote:
>> Adding `Toolkit::firePulse` is not making the tests stable. In fact I'm
>> getting new failure after adding this in
>> `testTextFlowInsertionIndexUsingMultipleEmojis()` test.
>>
>> TextFlowSurrogatePairInsertionIndexTest >
>> testText
On Mon, 22 May 2023 15:52:55 GMT, Kevin Rushforth wrote:
>>> could you please help me in understanding how `Util.runAndWait(() -> { });`
>>> help in this case
>>
>> After discussing the issue with @kevinrushforth it probably won't.
>>
>> The idea was to replace Thread.sleep() with something th
On Fri, 19 May 2023 15:41:50 GMT, Andy Goryachev wrote:
> For the purposes of this test, we could remove this sequence from the test, I
> think.
Removed red heart from the text sequence.
> Another variant is to add something like that to Util and use that in place
> of Thread.sleep(). This me
On Mon, 22 May 2023 14:21:33 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
> Since surrogate pairs are internally considered as 2 characters and text
> field is null in `HitInfo` when `getInsertionIndex` is invoked from
> `TextFlow`, wrong insertion index was returned.
>
> Updated code to calculate insertion index in `getHitInfo` method of
> `PrismTextLayout` class wh
> Since surrogate pairs are internally considered as 2 characters and text
> field is null in `HitInfo` when `getInsertionIndex` is invoked from
> `TextFlow`, wrong insertion index was returned.
>
> Updated code to calculate insertion index in `getHitInfo` method of
> `PrismTextLayout` class wh
On Fri, 19 May 2023 06:50:04 GMT, Karthik P K wrote:
> could you please help me in understanding how `Util.runAndWait(() -> { });`
> help in this case
After discussing the issue with @kevinrushforth it probably won't.
The idea was to replace Thread.sleep() with something that can work reliably
On Fri, 19 May 2023 11:28:10 GMT, Karthik P K wrote:
> value 10084
This character is a U+FE0F Variation Selector-16
https://codepoints.net/U+FE0F?lang=en
> This codepoint may change the appearance of the preceding character. If that
> is a symbol, dingbat or emoji, U+FE0F forces it to be rend
On Thu, 18 May 2023 17:03:53 GMT, Andy Goryachev wrote:
> please try adding firePulse(), and also please check on Windows and Linux.
> thanks!
I have run the tests in windows and linux. In windows, the red heart emoji
which is not a surrogate pair and represented by value 10084 gets displayed
On Thu, 18 May 2023 09:27:18 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Thu, 18 May 2023 09:27:18 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Thu, 18 May 2023 09:27:18 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Thu, 18 May 2023 09:27:18 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Thu, 18 May 2023 09:27:18 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Thu, 18 May 2023 17:04:57 GMT, Karthik P K wrote:
> `Toolkit.getToolkit().firePulse();` will not make any difference in system
> tests.
Once you added children to TextFlow, I think it needs another layout pass to
apply CSS and render the text. We could use delays, but it looks like
someti
On Thu, 18 May 2023 09:27:18 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Thu, 18 May 2023 09:27:18 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Thu, 18 May 2023 09:27:18 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Thu, 18 May 2023 09:27:18 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Thu, 18 May 2023 09:27:18 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Tue, 16 May 2023 10:32:00 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
> Since surrogate pairs are internally considered as 2 characters and text
> field is null in `HitInfo` when `getInsertionIndex` is invoked from
> `TextFlow`, wrong insertion index was returned.
>
> Updated code to calculate insertion index in `getHitInfo` method of
> `PrismTextLayout` class wh
On Tue, 16 May 2023 10:32:00 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Tue, 16 May 2023 10:31:57 GMT, Karthik P K wrote:
> I couldn't create the scenario to hit line 473. If you have any suggestions
> please let me know.
I can't seem to trigger this code path (I wonder if it's possible to hit it at
all). Anyway, the code there looks correct to me, even if thi
On Tue, 16 May 2023 10:32:00 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Mon, 15 May 2023 15:50:06 GMT, Andy Goryachev wrote:
>> Karthik P K has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Initialize insertion index in PrismTextLayout
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextL
> Since surrogate pairs are internally considered as 2 characters and text
> field is null in `HitInfo` when `getInsertionIndex` is invoked from
> `TextFlow`, wrong insertion index was returned.
>
> Updated code to calculate insertion index in `getHitInfo` method of
> `PrismTextLayout` class wh
On Mon, 15 May 2023 09:57:59 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion i
On Fri, 12 May 2023 21:05:27 GMT, Andy Goryachev wrote:
> Just by looking at the code, the code paths that leave `insertionIndex=-1` in
> `PrismTextLayout.getHitInfo()` are on lines: 431, 450(when run==null), 472
>
> If we can initialize insertionIndex to a positive value, then the buggy code
> Since surrogate pairs are internally considered as 2 characters and text
> field is null in `HitInfo` when `getInsertionIndex` is invoked from
> `TextFlow`, wrong insertion index was returned.
>
> Updated code to calculate insertion index in `getHitInfo` method of
> `PrismTextLayout` class wh
On Tue, 9 May 2023 05:10:25 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion in
On Tue, 9 May 2023 18:16:09 GMT, Andy Goryachev wrote:
>> Karthik P K has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Address code review
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java
> line 425:
>
On Tue, 9 May 2023 18:43:07 GMT, Andy Goryachev wrote:
> There is one question whether it's possible to always initialize
> HitInfo.insertionIndex and remove the secondary computation from
> HitInfo.getInsertionIndex()
>
> I am not clear under which conditions it is not possible to initialize,
On Tue, 9 May 2023 05:10:25 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion in
> Since surrogate pairs are internally considered as 2 characters and text
> field is null in `HitInfo` when `getInsertionIndex` is invoked from
> `TextFlow`, wrong insertion index was returned.
>
> Updated code to calculate insertion index in `getHitInfo` method of
> `PrismTextLayout` class wh
On Mon, 8 May 2023 16:33:50 GMT, Andy Goryachev wrote:
> Does not seem to work correctly still - the insertion index in TextFlow
> should not point in between surrogate pair of characters that comprise an
> emoji. Basically, it should be exactly the same as with Text.hitInfo():
>
This conditio
On Mon, 8 May 2023 12:22:17 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion in
On Mon, 8 May 2023 12:22:17 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion in
On Mon, 8 May 2023 12:22:17 GMT, Karthik P K wrote:
>> Since surrogate pairs are internally considered as 2 characters and text
>> field is null in `HitInfo` when `getInsertionIndex` is invoked from
>> `TextFlow`, wrong insertion index was returned.
>>
>> Updated code to calculate insertion in
> Since surrogate pairs are internally considered as 2 characters and text
> field is null in `HitInfo` when `getInsertionIndex` is invoked from
> `TextFlow`, wrong insertion index was returned.
>
> Updated code to calculate insertion index in `getHitInfo` method of
> `PrismTextLayout` class wh
On Wed, 26 Apr 2023 19:47:43 GMT, Andy Goryachev wrote:
> > I'm also seeing same issue
>
> I wonder what could be the difference? text size?
Text size is one difference.
One more observation I made is that if the emojis and text in the TextFlow are
separate text nodes then this fix works. If o
On Wed, 26 Apr 2023 06:00:19 GMT, Karthik P K wrote:
> I'm also seeing same issue
I wonder what could be the difference? text size?
-
PR Comment: https://git.openjdk.org/jfx/pull/1091#issuecomment-1523955682
On Fri, 21 Apr 2023 16:29:46 GMT, Andy Goryachev wrote:
> could you please check with the Monkey Tester? I could be doing something
> wrong.
I checked above scenario in Monkey tester and I'm also seeing same issue. But
in a simple test app which I created separately, it looks to be working. I
On Fri, 14 Apr 2023 07:35:33 GMT, Karthik P K wrote:
> Since surrogate pairs are internally considered as 2 characters and text
> field is null in `HitInfo` when `getInsertionIndex` is invoked from
> `TextFlow`, wrong insertion index was returned.
>
> Updated code to calculate insertion index
On Fri, 14 Apr 2023 07:35:33 GMT, Karthik P K wrote:
> Since surrogate pairs are internally considered as 2 characters and text
> field is null in `HitInfo` when `getInsertionIndex` is invoked from
> `TextFlow`, wrong insertion index was returned.
>
> Updated code to calculate insertion index
Since surrogate pairs are internally considered as 2 characters and text field
is null in `HitInfo` when `getInsertionIndex` is invoked from `TextFlow`, wrong
insertion index was returned.
Updated code to calculate insertion index in `getHitInfo` method of
`PrismTextLayout` class when `hitTest`
70 matches
Mail list logo
