Delete dialog in file attachements is not focused

[stefan-kolb]

I cannot edit or focus it. Instead of one, two dialogs spawn in the background.

Might be caused by #3690

[stefan-kolb]

java.lang.NullPointerException
	at com.sun.javafx.webkit.prism.WCPageBackBufferImpl.validate(WCPageBackBufferImpl.java:117)
	at com.sun.webkit.WebPage.paint(WebPage.java:674)
	at com.sun.javafx.sg.prism.web.NGWebView.renderContent(NGWebView.java:96)
	at com.sun.javafx.sg.prism.NGNode.doRender(NGNode.java:2053)
	at com.sun.javafx.sg.prism.NGNode.render(NGNode.java:1945)
	at com.sun.javafx.sg.prism.NGGroup.renderContent(NGGroup.java:235)
	at com.sun.javafx.sg.prism.NGRegion.renderContent(NGRegion.java:576)
	at com.sun.javafx.sg.prism.NGNode.doRender(NGNode.java:2053)
	at com.sun.javafx.sg.prism.NGNode.render(NGNode.java:1945)
	at com.sun.javafx.sg.prism.NGGroup.renderContent(NGGroup.java:235)
	at com.sun.javafx.sg.prism.NGRegion.renderContent(NGRegion.java:576)
	at com.sun.javafx.sg.prism.NGNode.renderForClip(NGNode.java:2294)
	at com.sun.javafx.sg.prism.NGNode.renderRectClip(NGNode.java:2188)
	at com.sun.javafx.sg.prism.NGNode.renderClip(NGNode.java:2214)
	at com.sun.javafx.sg.prism.CacheFilter.impl_renderNodeToCache(CacheFilter.java:671)
	at com.sun.javafx.sg.prism.CacheFilter.render(CacheFilter.java:575)
	at com.sun.javafx.sg.prism.NGNode.renderCached(NGNode.java:2358)
	at com.sun.javafx.sg.prism.NGNode.doRender(NGNode.java:2044)
	at com.sun.javafx.sg.prism.NGNode.render(NGNode.java:1945)
	at com.sun.javafx.sg.prism.NGGroup.renderContent(NGGroup.java:235)
	at com.sun.javafx.sg.prism.NGRegion.renderContent(NGRegion.java:576)
	at com.sun.javafx.sg.prism.NGNode.doRender(NGNode.java:2053)
	at com.sun.javafx.sg.prism.NGNode.render(NGNode.java:1945)
	at com.sun.javafx.tk.quantum.ViewPainter.doPaint(ViewPainter.java:477)
	at com.sun.javafx.tk.quantum.ViewPainter.paintImpl(ViewPainter.java:330)
	at com.sun.javafx.tk.quantum.UploadingPainter.run(UploadingPainter.java:134)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:308)
	at com.sun.javafx.tk.RenderJob.run(RenderJob.java:58)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at com.sun.javafx.tk.quantum.QuantumRenderer$PipelineRunnable.run(QuantumRenderer.java:125)
	at java.lang.Thread.run(Thread.java:745)

[stefan-kolb]

Looks like this is a general problem for all dialogs for me.

[stefan-kolb]

Using the newest JDk does not seem to help here 😭

[stefan-kolb]

https://bugs.openjdk.java.net/browse/JDK-8133344

I have also no idea why something with a WebView is triggered when a FXDialog is started...

[tobiasdiez]

Does the problem also occurs on the maintable-beta branch?

[Siedlerchr]

When I read the bug report and the linked issue right it seems it could be due to the high screen resolution

[bdach]

Since I was the one to last touch that particular dialog, I decided to do some digging as well.

I was able to reproduce the duplicate windows, although for me they are extremely nondeterministic. However I have some suspicions that this might be timing-dependent, since I am having a way easier time reproducing this bug after a fresh compilation, and the issue seems to pop up pretty much only the first time the dialog is shown, if it does at all.

I think that the event filter for the dialog might be susceptible to repeated triggers of the Delete key binding. My attempts at verification were finicky at best, however (I tried to produce multiple keypresses by breakpointing inside the handler and trying to race the dialog taking over the focus when resuming, or by pressing Delete in quick succession on two keyboards, and those attempts sometimes succeeded)

As for the focus loss, unfortunately I couldn't observe that happening on my machine.

[Siedlerchr]

@bdach That sounds like a possible reason. Could you please try to test it with KeyEvent.KeyDown instead of KeyEvent.Any?

[bdach]

Okay, so I found an absolutely insane way of reproducing duplicate windows without fail. I did

$ xset r rate 5 255

to exploit the keyboard autorepeat mechanism. The first argument is the delay from the first keypress after which autorepeat is engaged, and the second one is the rate of repeat keypresses.

Unfortunately KeyEvent.KEY_PRESSED does not fix the problem. The root cause seems to be the fact that the main JabRef window can take multiple Delete presses before FXDialogService displays the first dialog and takes focus, which means we now have multiple calls to the dialog service, each of which displays its own copy:

This unfortunately looks quite nasty to fix, and to fix properly I'd imagine some synchronization method would have to be used in the dialog service.

[stefan-kolb]

I think the focus problem is caused by the QHD display resolution, just checked it on my other PC 😢
Also, if I resize Jabref, the entry preview tends to dissapear again with the newq entry editor (size is 0 then or so).

[bdach]

So I tried using a semaphore to fix the duplicate dialogs, using the following example patch:

--- src/main/java/org/jabref/gui/FXDialogService.java	(date 1518047957000)
+++ src/main/java/org/jabref/gui/FXDialogService.java	(revision )
@@ -5,6 +5,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
+import java.util.concurrent.Semaphore;
 import java.util.stream.Collectors;
 
 import javafx.concurrent.Task;
@@ -38,6 +39,8 @@
  */
 public class FXDialogService implements DialogService {
 
+    private final Semaphore semaphore = new Semaphore(1);
+
     private static FXDialog createDialog(AlertType type, String title, String content) {
         FXDialog alert = new FXDialog(type, title, true);
         alert.setHeaderText(null);
@@ -112,9 +115,12 @@
     @Override
     public Optional<ButtonType> showCustomButtonDialogAndWait(AlertType type, String title, String content,
             ButtonType... buttonTypes) {
+        if (!semaphore.tryAcquire()) return Optional.empty();
         FXDialog alert = createDialog(type, title, content);
         alert.getButtonTypes().setAll(buttonTypes);
-        return alert.showAndWait();
+        Optional<ButtonType> buttonType = alert.showAndWait();
+        semaphore.release();
+        return buttonType;
     }
 
     @Override

and it seems to fix it (as in I cannot reproduce the issue anymore, even with the fast key repetition). I don't very much like the change, however, since it would potentially require patching all dialog methods, but it's obviously better than forcing the callers to take the semaphore themselves.

There could be additional issues caused by returning the empty optional early (even though the displayed dialog is pretty much guaranteed to return last).

I can also confirm that other dialogs are vulnerable to the same bug (e.g. the file open dialog referenced above), but I think there's another factor at play there, since the manifestation is slightly different (instead of having multiple dialogs open at once, multiple dialogs are opened in sequence). I am going to investigate that next.

[Siedlerchr]

Thanks for looking into this, but I would not spend too much time on this as this is probably related to the swing /JavaFX threading. You could try to test if it still happens in the maintable beta branch where the maintable is already converted to JavaFX

[bdach]

Yeah, I'm hitting a wall at swing/awt here. For the file open/save dialogs, it seems like ActionListener#actionPerformed(java.awt.event.ActionEvent) is getting called too many times when keyboard events occur too quickly.

Both issues still persist on maintable-beta, however – I managed to produce double dialogs manually.