Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Find/replace overlay: does not take focus after detach #2246

Open
Tracked by #2021
jukzi opened this issue Sep 6, 2024 · 4 comments · May be fixed by #2254
Open
Tracked by #2021

Find/replace overlay: does not take focus after detach #2246

jukzi opened this issue Sep 6, 2024 · 4 comments · May be fixed by #2254
Labels
bug Something isn't working regression

Comments

@jukzi
Copy link
Contributor

jukzi commented Sep 6, 2024

on Windows OS:

open an editor
detach it (drag to another screen or "detach" in the tabs context menu)
press strg+f -> opens search
enter "hallo"
-> "hallo" edits the document. it is supposed to be written in the search dialog

After detaaching mutliple times it is sometimes not possible to open Find/replace overlay at all.
Errors found in the error log:

org.eclipse.swt.SWTException: Widget is disposed
	at org.eclipse.swt.SWT.error(SWT.java:4922)
	at org.eclipse.swt.SWT.error(SWT.java:4837)
	at org.eclipse.swt.SWT.error(SWT.java:4808)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:500)
	at org.eclipse.swt.widgets.Widget.getDisplay(Widget.java:622)
	at org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay$TargetPartVisibilityHandler.partHidden(FindReplaceOverlay.java:271)

Caused by: org.eclipse.swt.SWTException: Widget is disposed
	at org.eclipse.swt.SWT.error(SWT.java:4922)
	at org.eclipse.swt.SWT.error(SWT.java:4837)
	at org.eclipse.swt.SWT.error(SWT.java:4808)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:500)
	at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:419)
	at org.eclipse.swt.widgets.Control.setBackground(Control.java:3071)
	at org.eclipse.ui.internal.findandreplace.overlay.AccessibleToolBar.setBackground(AccessibleToolBar.java:63)
	at org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay.applyOverlayColors(FindReplaceOverlay.java:388)
	at org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay.open(FindReplaceOverlay.java:365)

org.eclipse.swt.SWTException: Widget is disposed
	at org.eclipse.swt.SWT.error(SWT.java:4922)
	at org.eclipse.swt.SWT.error(SWT.java:4837)
	at org.eclipse.swt.SWT.error(SWT.java:4808)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:500)
	at org.eclipse.swt.widgets.Widget.getDisplay(Widget.java:622)
	at org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay.asyncUpdatePlacementAndVisibility(FindReplaceOverlay.java:212)
	at org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay.lambda$0(FindReplaceOverlay.java:207)
	at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:247)

org.eclipse.swt.SWTException: Widget is disposed
	at org.eclipse.swt.SWT.error(SWT.java:4922)
	at org.eclipse.swt.SWT.error(SWT.java:4837)
	at org.eclipse.swt.SWT.error(SWT.java:4808)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:500)
	at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:419)
	at org.eclipse.swt.widgets.Control.getParent(Control.java:1449)
	at org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay.isInvalidTargetShell(FindReplaceOverlay.java:886)
	at org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay.lambda$22(FindReplaceOverlay.java:849)

java.lang.NullPointerException: Cannot invoke "org.eclipse.swt.widgets.Shell.setVisible(boolean)" because the return value of "org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay.getShell()" is null
	at org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay.updatePlacementAndVisibility(FindReplaceOverlay.java:844)

@jukzi jukzi added bug Something isn't working regression labels Sep 6, 2024
@HeikoKlare
Copy link
Contributor

Thank you for reporting this. The NPE should be fixed via #2247.

I can also reproduce some of the other exceptions. But to do so, I have to reattach an editor to another shell (so that the shell of the detached editor is disposed). I cannot reproduce the behavior of improper focus when the editor is detached. When following the described use case (detach editor, open overlay, enter something), the overlay is properly opened and focused and receives the input for me.

@jukzi
Copy link
Contributor Author

jukzi commented Sep 9, 2024

i can reproduce detach focus every time. here is a video:
italic

@jukzi
Copy link
Contributor Author

jukzi commented Sep 9, 2024

and still get Widget is disposed after multiple detaches:

!ENTRY org.eclipse.ui 4 0 2024-09-09 14:34:46.531
!MESSAGE Unhandled event loop exception
!STACK 0
org.eclipse.swt.SWTException: Widget is disposed
	at org.eclipse.swt.SWT.error(SWT.java:4922)
	at org.eclipse.swt.SWT.error(SWT.java:4837)
	at org.eclipse.swt.SWT.error(SWT.java:4808)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:500)
	at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:419)
	at org.eclipse.swt.widgets.Control.getParent(Control.java:1449)
	at org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay.isInvalidTargetShell(FindReplaceOverlay.java:895)
	at org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay.lambda$24(FindReplaceOverlay.java:858)
	at org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay.lambda$8(FindReplaceOverlay.java:215)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:132)
	at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4099)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3715)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1151)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1042)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:153)
	at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:639)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:546)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:173)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:208)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:143)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:109)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:439)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:271)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:668)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:605)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1481)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1454)

!ENTRY org.eclipse.ui 4 0 2024-09-09 14:34:46.538
!MESSAGE Unhandled event loop exception
!STACK 0
org.eclipse.swt.SWTException: Widget is disposed
	at org.eclipse.swt.SWT.error(SWT.java:4922)
	at org.eclipse.swt.SWT.error(SWT.java:4837)
	at org.eclipse.swt.SWT.error(SWT.java:4808)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:500)
	at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:419)
	at org.eclipse.swt.widgets.Control.getParent(Control.java:1449)
	at org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay.isInvalidTargetShell(FindReplaceOverlay.java:895)
	at org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay.lambda$24(FindReplaceOverlay.java:858)
	at org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay.lambda$8(FindReplaceOverlay.java:215)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:40)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:132)
	at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4099)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3715)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1151)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1042)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:153)
	at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:639)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:546)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:173)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:208)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:143)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:109)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:439)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:271)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:668)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:605)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1481)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1454)

@HeikoKlare
Copy link
Contributor

From my understanding, these exception occur when reattaching. I was not able to produce them by detaching so far. Maybe I am doing something differently.

I was able to produce some detach focus issue, but in my case it is different from your behavior: when pressing CTRL+F in the detached window, the find/replace is opened for the active editor in the original shell rather than the in the detached shell, and also that one receives focus. But that is an existing problem of the FindReplaceAction, which also happens when using the existing dialog instead of the overlay.

I would like to push #2254 forward, as it is supposed to solve several issues (including this one) and make the UI integration way cleaner, as it gets rid of the additional shell instantiated for the overlay. In case that solution does not prove to work out, I will come back to addressing specifically this issue.

HeikoKlare added a commit to HeikoKlare/eclipse.platform.ui that referenced this issue Oct 3, 2024
…-platform#2099

The FindReplaceOverlay is currently realized as a separate shell (more
precisely, a JFace Dialog), which is placed at a proper position on top
of the workbench shell. This has some drawback:
- It has to manually adapt to movements of the parent shell or the
target part/widget
- It has to manually hide and show depending on visibility changes of
the target part/widget
- It does not follow events of the target immediately, i.e., movements
are always some milliseconds behind, minimize/maximize
operations/animations are not synchronous etc.
- It does not locate properly when the platform uses Wayland, as manual
shell positioning is not possible there

This change replaces the dialog-based implementation of the
FindReplaceOverlay with an in-place composite-based implementation. A
composite is created in the target widget and placed relative to this
composite. In consequence, the overlay automatically follows all move,
resize, hide/show operations of the target widget.

Fixes eclipse-platform/eclipse.platform.swt#1447

Fixes eclipse-platform#2099

Fixes eclipse-platform#2246
HeikoKlare added a commit to HeikoKlare/eclipse.platform.ui that referenced this issue Oct 3, 2024
…-platform#2099

The FindReplaceOverlay is currently realized as a separate shell (more
precisely, a JFace Dialog), which is placed at a proper position on top
of the workbench shell. This has some drawback:
- It has to manually adapt to movements of the parent shell or the
target part/widget
- It has to manually hide and show depending on visibility changes of
the target part/widget
- It does not follow events of the target immediately, i.e., movements
are always some milliseconds behind, minimize/maximize
operations/animations are not synchronous etc.
- It does not locate properly when the platform uses Wayland, as manual
shell positioning is not possible there

This change replaces the dialog-based implementation of the
FindReplaceOverlay with an in-place composite-based implementation. A
composite is created in the target widget and placed relative to this
composite. In consequence, the overlay automatically follows all move,
resize, hide/show operations of the target widget.

Fixes eclipse-platform/eclipse.platform.swt#1447

Fixes eclipse-platform#2099

Fixes eclipse-platform#2246
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants