[Bug]: NPE preventing HTML 5 dialogs from submitting #4048

kwvanderlinde · 2023-05-09T21:40:17Z

Describe the Bug

In some circumstances, after reopening an HTML5 dialog, the dialog can no longer be closed via a submit button.

To Reproduce

Create a token "Lib:Dragon"
Add a macro to the token named "Action" with these contents:
```
[h: broadcast(macro.args)]
```

Add a second macro the token named "Open Dialog" with these contents:

[r, dialog5("Dialog", "input=1;closebutton=1"):{
<!DOCTYPE html>
<html lang="en">
 <head></head>
 <body>
 	<form action="macro://Action@Lib:Dragon/none/Impersonated?" method="json">
 	<button type="submit">Done</button>
 </body>
</html>
}]

Click the "Open Dialog" macro to be presented the simple dialog.
Click the "Submit" button to close the dialog and see some output in the chat window from the "Action" macro.
Reopen the campaign in the same instance of MapTool (e.g. via File > Recent Campaigns).
Click the "Open Dialog" macro to open the dialog again
Click the "Submit" button in the dialog. Notice the dialog stays open and there is no output in chat.
Close the dialog via the "x" button.
Change the dialog name in the "Open Dialog" macro and open it again
Notice that the newly named dialog works as expected ... for now.

Expected Behaviour

The dialog form should be submitted, handled via the macro referenced in the form action, and the dialog should then be closed.

Screenshots

No response

MapTool Info

develop (f0c70b5)

Desktop

Linux Mint 21.1

Additional Context

This is new on the develop branch and does not affect the 1.13 release.

Reopening the campaign is not strictly necessary, but I have found it tends to be more consistently reproducable that way.

The failure to submit is accompanied by this exception:

Exception in thread "JavaFX Application Thread" java.lang.NullPointerException
	at javafx.web/com.sun.webkit.WebPage.twkProcessMouseEvent(Native Method)
	at javafx.web/com.sun.webkit.WebPage.dispatchMouseEvent(WebPage.java:857)
	at javafx.web/javafx.scene.web.WebView.processMouseEvent(WebView.java:1098)
	at javafx.web/javafx.scene.web.WebView.lambda$registerEventHandlers$3(WebView.java:1224)
	at javafx.base/com.sun.javafx.event.CompositeEventHandler$NormalEventHandlerRecord.handleBubblingEvent(CompositeEventHandler.java:247)
	at javafx.base/com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:80)
	at javafx.base/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:232)
	at javafx.base/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:189)
	at javafx.base/com.sun.javafx.event.CompositeEventDispatcher.dispatchBubblingEvent(CompositeEventDispatcher.java:59)
	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:56)
	at javafx.base/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
	at javafx.base/com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
	at javafx.base/com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:54)
	at javafx.base/javafx.event.Event.fireEvent(Event.java:198)
	at javafx.graphics/javafx.scene.Scene$MouseHandler.process(Scene.java:3980)
	at javafx.graphics/javafx.scene.Scene.processMouseEvent(Scene.java:1890)
	at javafx.graphics/javafx.scene.Scene$ScenePeerListener.mouseEvent(Scene.java:2704)
	at javafx.graphics/com.sun.javafx.tk.quantum.EmbeddedScene.lambda$mouseEvent$4(EmbeddedScene.java:288)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:400)
	at javafx.graphics/com.sun.javafx.tk.quantum.EmbeddedScene.lambda$mouseEvent$5(EmbeddedScene.java:281)
	at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(PlatformImpl.java:456)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:400)
	at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$11(PlatformImpl.java:455)
	at javafx.graphics/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)
	at javafx.graphics/com.sun.glass.ui.gtk.GtkApplication._runLoop(Native Method)
	at javafx.graphics/com.sun.glass.ui.gtk.GtkApplication.lambda$runLoop$11(GtkApplication.java:316)
	at java.base/java.lang.Thread.run(Thread.java:1623)

The text was updated successfully, but these errors were encountered:

kwvanderlinde · 2023-08-16T16:17:57Z

Bisected this to 1ddaa7b

kwvanderlinde · 2023-08-16T17:42:38Z

This issue is not unique to submit buttons, . The same error happens for plain buttons and anchors (with or without an href), and probably plenty of other interactable elements (not <select> though...).

Unfortunately I haven't had much luck pinning down the root cause. The failure is in native code for some invisible resource, and so far everything I can see in Javaland looks fine. After reverting JavaFX to version 18, I could no longer reproduce the issue, while on JavaFX 19 and 20 it is pretty consistently showing up. So one solution would be to downgrade.

I tried out some different code changes as well to see if we can work around the problem and stay on JavaFX 20. One thing that worked was to discard the HTMLJFXPanel when closing the dialog, then create a fresh one when reopening the dialog again. Scroll reset was a casualty, but that could be fixed with a bit more effort. Have to try it yet, but I think the WebView is the key here, so we could keep the panel around but have the contained HTMLWebViewManager use a fresh WebView when closed/reopened. That approach would naturally preserve scroll reset.

Another thing I noticed was that the HTMLWebViewManager loads about:blank when closing the dialog. If I remove that, I can no longer recreate the issue. So this could be another option for addressing the issue (assuming we can find another way to accomplish its purpose). I don't really want to do that though, it mainly just indicates to me that we're probably not the ones at fault here, rather JavaFX is fumbling something.

I also added some threading checks to make sure all methods of HTMLWebViewManager are running where they should be, especially since there is some JavaFx / Swing interop there. There was a little bit of questionable behaviour, but even addressing that did nothing to avoid the current issue.

Azhrei · 2023-08-16T21:25:40Z

It’s a shame to hear it’s a regression in JFX 19 and 20 (or appears to be, at least). JFX 20 fixes an issue where the jQuery-UI library’s Tabs feature wasn’t working. (Same code work on v20 but not v18. I didn’t dig into why.)

kwvanderlinde · 2023-08-17T00:00:08Z

Progress update:

I found two different low-level aspect that seems to interact to create this issue:

As mentioned above, loading about:blank when closing the dialog.
Registering Java objects in the DOM to listen for submits.

Removing either will avoid the NPE from occuring (of course, removing the latter also means submits don't work at all 😅. But this is just debugging progress, not a suggestion for implementation).

I don't think we're doing anything wrong there. My working theory is that JavaFX is just losing / messing up the association between the Java listeners and their native peers when the page changes, possibly because it's cached the Java listener but doesn't invalidate the listener's peer when a new page is loaded 🤷 Whether or not this is accurate, the behaviour I'm seeing suggests we may be able to work around it by registering listener in JS rather than Java, and forward the events via our bridge code as that stuff seems to be working fine. Have yet to test this though.

cwisniew · 2023-08-17T00:15:31Z

Removing either will avoid the NPE from occuring (_of course, removing the latter also means submits don't work at all 😅. But this is just debugging progress, not a suggestion for implementation).

I have found another non useable work around... If I run it from the IDE in debug mode it never fails for me... I hate when that happens

kwvanderlinde · 2023-08-17T15:12:29Z

I've found that a fix went in for JavaFX 19 to allow garbage collection of EventTarget listeners: JDK-8088420. Quite possible that change included some undesirable behaviour.

I'm going to try reproduce this in a minimal example, and if I can I will report it to the JavaFX team. Then I'll keep looking for a workaround.

kwvanderlinde · 2023-08-24T21:06:58Z

I've found the key to be whether a GC cycle runs while the page is unloaded. So waiting a bit before reopening the dialog will more reliably result in a breakage. This also explains why reloading the campaign made it more consistent for me originally.

I've reproduced this in a minimal example that I will send to the JavaFX team. In the meantime, a very simple workaround has presented itself, so I'll put a PR together for that.

kwvanderlinde · 2023-09-11T20:59:53Z

Finally my bug report got added to the JDK bug tracker: JDK-8315915.

kwvanderlinde · 2023-09-12T17:53:10Z

We're seeing redundant events being fired since the attempted fix, e.g., when clicking macro links. Will have to look into how we can avoid this.

kwvanderlinde · 2023-11-29T23:55:58Z

Looks like #4285 has had other adverse effects. My mistaken assumption was that our MutationObserver would be called for all children added to the DOM, because subtree is set. This actually does not happen, meaning "deeply" nested nodes can be missed.

kwvanderlinde · 2023-11-30T00:03:26Z

Update on JDK-8315915: it looks like it will be fixed in JavaFX 22. Did some very quick testing and it seems to be the case.

kwvanderlinde · 2024-04-01T22:20:54Z

Closing this off as there was confirmation in Discord at the time that this was fixed, and there haven't been further reports to the contrary.

kwvanderlinde added the bug label May 9, 2023

kwvanderlinde self-assigned this Aug 15, 2023

kwvanderlinde added this to MapTool 1.14.0 Aug 15, 2023

kwvanderlinde moved this to In Progress in MapTool 1.14.0 Aug 15, 2023

kwvanderlinde mentioned this issue Aug 24, 2023

Work around the JavaFX NPE bug causing HTML5 listeners to not be called #4259

Merged

cwisniew closed this as completed in #4259 Aug 24, 2023

github-project-automation bot moved this from In Progress to Done in MapTool 1.14.0 Aug 24, 2023

kwvanderlinde moved this from Done to Needs Testing in MapTool 1.14.0 Aug 31, 2023

kwvanderlinde reopened this Sep 12, 2023

kwvanderlinde mentioned this issue Sep 13, 2023

Remove child node enumeration when registering HTML5 event listeners #4285

Merged

kwvanderlinde mentioned this issue Nov 30, 2023

Go back to enumerating HTML5 child nodes when added, and adding event listeners to them #4508

Merged

kwvanderlinde closed this as completed Apr 1, 2024

github-project-automation bot moved this from Needs Testing to Done in MapTool 1.14.0 Apr 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bug]: NPE preventing HTML 5 dialogs from submitting #4048

[Bug]: NPE preventing HTML 5 dialogs from submitting #4048

kwvanderlinde commented May 9, 2023

kwvanderlinde commented Aug 16, 2023

kwvanderlinde commented Aug 16, 2023

Azhrei commented Aug 16, 2023

kwvanderlinde commented Aug 17, 2023

cwisniew commented Aug 17, 2023

kwvanderlinde commented Aug 17, 2023 •

edited

Loading

kwvanderlinde commented Aug 24, 2023

kwvanderlinde commented Sep 11, 2023

kwvanderlinde commented Sep 12, 2023

kwvanderlinde commented Nov 29, 2023

kwvanderlinde commented Nov 30, 2023

kwvanderlinde commented Apr 1, 2024

[Bug]: NPE preventing HTML 5 dialogs from submitting #4048

[Bug]: NPE preventing HTML 5 dialogs from submitting #4048

Comments

kwvanderlinde commented May 9, 2023

Describe the Bug

To Reproduce

Expected Behaviour

Screenshots

MapTool Info

Desktop

Additional Context

kwvanderlinde commented Aug 16, 2023

kwvanderlinde commented Aug 16, 2023

Azhrei commented Aug 16, 2023

kwvanderlinde commented Aug 17, 2023

cwisniew commented Aug 17, 2023

kwvanderlinde commented Aug 17, 2023 • edited Loading

kwvanderlinde commented Aug 24, 2023

kwvanderlinde commented Sep 11, 2023

kwvanderlinde commented Sep 12, 2023

kwvanderlinde commented Nov 29, 2023

kwvanderlinde commented Nov 30, 2023

kwvanderlinde commented Apr 1, 2024

kwvanderlinde commented Aug 17, 2023 •

edited

Loading