On Fri, 5 Aug 2022 07:34:43 GMT, Marius Hanl <mh...@openjdk.org> wrote:
>> Florian Kirmaier has updated the pull request with a new target base due to >> a merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains three additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'origjfx/master' into >> JDK-8269921-textflow-bug >> - JDK-8269921 >> Added a copyright header >> - JDK-8269921 >> Fixing a crash related to Text and TextFlow with bounds listeners > > I got this problem as well today. NPE as `runs` is null in line `359`. Does > it make sense to first 'resolve' this by adding a simple null check and later > we may try your other change which removes the call to `getParent().layout()`? > > Edit: I also got another interesting NPE in `PrismTextLayout`: > > java.lang.NullPointerException: Cannot read the array length because > "this.runs" is null > at > com.sun.javafx.text.PrismTextLayout.addTextRun(PrismTextLayout.java:770) > at com.sun.javafx.text.GlyphLayout.addTextRun(GlyphLayout.java:140) > at com.sun.javafx.text.GlyphLayout.breakRuns(GlyphLayout.java:210) > at com.sun.javafx.text.PrismTextLayout.buildRuns(PrismTextLayout.java:785) > at com.sun.javafx.text.PrismTextLayout.layout(PrismTextLayout.java:1036) > at > com.sun.javafx.text.PrismTextLayout.ensureLayout(PrismTextLayout.java:223) > at com.sun.javafx.text.PrismTextLayout.getBounds(PrismTextLayout.java:246) > at javafx.scene.text.Text.getLogicalBounds(Text.java:432) > at javafx.scene.text.Text.doComputeGeomBounds(Text.java:1187) > at javafx.scene.text.Text$1.doComputeGeomBounds(Text.java:149) > at > com.sun.javafx.scene.shape.TextHelper.computeGeomBoundsImpl(TextHelper.java:90) > at com.sun.javafx.scene.NodeHelper.computeGeomBounds(NodeHelper.java:116) > at javafx.scene.Node.updateGeomBounds(Node.java:3818) > at javafx.scene.Node.getGeomBounds(Node.java:3780) > at javafx.scene.Node.updateBounds(Node.java:776) > at javafx.scene.Parent.updateBounds(Parent.java:1833) > ... > <<pulse>> > The simple null check helps in this case. But sometimes I see the exception, > posted by @Maran23 . So the root cause is probably not fixed - but the > symptoms are. I guess, this PR is then a good progress. Maybe it's not good progress. You've investigated the issue, and have identified a likely root cause. Not fixing the root cause just means that it most likely won't be fixed for years to come, and the insights you've accumulated will fade. I think we should just fix the root cause. If some application breaks as a result of this, not much harm is done: we'd gain valuable insight into a real-world situation that depends on the current behavior, and can then decide whether to revert the fix, or provide a viable alternative solution (for users) that accomplishes the same goal. ------------- PR: https://git.openjdk.org/jfx/pull/564
