Find/replace overlay: replace shell with integrated composite #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 #2099

Author: HeikoKlare

bundles/org.eclipse.ui.workbench.texteditor/icons/full/elcl16/close_replace.png

@@ -429,7 +429,7 @@ private void showOverlayInEditor() {
 		overlay.setPositionToTop(shouldPositionOverlayOnTop());
 
 		hookDialogPreferenceListener();
-		overlay.getShell().addDisposeListener(__ -> removeDialogPreferenceListener());
+		overlay.addDisposeListener(__ -> removeDialogPreferenceListener());
 	}
 
 	@Override

bundles/org.eclipse.ui.workbench.texteditor/icons/full/elcl16/[email protected]

@@ -119,7 +119,6 @@ public void testDisableWholeWordIfRegEx() {
 		dialog.assertEnabled(SearchOptions.REGEX);
 		dialog.assertEnabled(SearchOptions.WHOLE_WORD);
 
-		dialog.getFindReplaceLogic().updateTarget(fTextViewer.getFindReplaceTarget(), false);
 		dialog.select(SearchOptions.WHOLE_WORD);
 		dialog.select(SearchOptions.REGEX);
 		dialog.assertEnabled(SearchOptions.REGEX);
@@ -161,7 +160,7 @@ public void testShiftEnterReversesSearchDirection() {
 		dialog.select(SearchOptions.INCREMENTAL);
 		dialog.setFindText("line");
 		ensureHasFocusOnGTK();
-		IFindReplaceTarget target= dialog.getTarget();
+		IFindReplaceTarget target= getFindReplaceTarget();
 
 		assertEquals(0, (target.getSelection()).x);
 		assertEquals(4, (target.getSelection()).y);
@@ -190,7 +189,7 @@ public void testChangeInputForIncrementalSearch() {
 		dialog.select(SearchOptions.INCREMENTAL);
 
 		dialog.setFindText("lin");
-		IFindReplaceTarget target= dialog.getTarget();
+		IFindReplaceTarget target= getFindReplaceTarget();
 		assertEquals(0, (target.getSelection()).x);
 		assertEquals(dialog.getFindText().length(), (target.getSelection()).y);
 
@@ -206,7 +205,7 @@ public void testFindWithWholeWordEnabledWithMultipleWords() {
 		dialog.select(SearchOptions.WHOLE_WORD);
 		dialog.select(SearchOptions.WRAP);
 		dialog.setFindText("two");
-		IFindReplaceTarget target= dialog.getTarget();
+		IFindReplaceTarget target= getFindReplaceTarget();
 		assertEquals(0, (target.getSelection()).x);
 		assertEquals(3, (target.getSelection()).y);
 
@@ -227,7 +226,7 @@ public void testRegExSearch() {
 		dialog.select(SearchOptions.REGEX);
 		dialog.setFindText("(a|bc)");
 
-		IFindReplaceTarget target= dialog.getTarget();
+		IFindReplaceTarget target= getFindReplaceTarget();
 		dialog.simulateKeyboardInteractionInFindInputField(SWT.CR, false);
 		assertEquals(0, (target.getSelection()).x);
 		assertEquals(1, (target.getSelection()).y);
@@ -374,4 +373,8 @@ protected TextViewer getTextViewer() {
 		return fTextViewer;
 	}
 
+	protected final IFindReplaceTarget getFindReplaceTarget() {
+		return fTextViewer.getFindReplaceTarget();
+	}
+
 }

bundles/org.eclipse.ui.workbench.texteditor/icons/full/elcl16/open_replace.png

@@ -12,15 +12,11 @@
 
 import org.eclipse.swt.widgets.Widget;
 
-import org.eclipse.jface.text.IFindReplaceTarget;
-
 /**
  * Wraps UI access for different find/replace UIs
  */
 public interface IFindReplaceUIAccess {
 
-	IFindReplaceTarget getTarget();
-
 	void closeAndRestore();
 
 	void close();
@@ -47,8 +43,6 @@ public interface IFindReplaceUIAccess {
 
 	Widget getButtonForSearchOption(SearchOptions option);
 
-	IFindReplaceLogic getFindReplaceLogic();
-
 	void performReplaceAll();
 
 	void performReplace();

bundles/org.eclipse.ui.workbench.texteditor/icons/full/elcl16/[email protected]

@@ -46,7 +46,7 @@ public OverlayAccess openUIFromTextViewer(TextViewer viewer) {
 		Accessor actionAccessor= new Accessor(getFindReplaceAction(), FindReplaceAction.class);
 		actionAccessor.invoke("showOverlayInEditor", null);
 		Accessor overlayAccessor= new Accessor(actionAccessor.get("overlay"), "org.eclipse.ui.internal.findandreplace.overlay.FindReplaceOverlay", getClass().getClassLoader());
-		return new OverlayAccess(overlayAccessor);
+		return new OverlayAccess(getFindReplaceTarget(), overlayAccessor);
 	}
 
 	@Test
@@ -55,7 +55,7 @@ public void testDirectionalSearchButtons() {
 		OverlayAccess dialog= getDialog();
 
 		dialog.setFindText("line");
-		IFindReplaceTarget target= dialog.getTarget();
+		IFindReplaceTarget target= getFindReplaceTarget();
 
 		assertEquals(0, (target.getSelection()).x);
 		assertEquals(4, (target.getSelection()).y);
@@ -89,14 +89,14 @@ public void testDirectionalSearchButtons() {
 	public void testIncrementalSearchUpdatesAfterChangingOptions() {
 		initializeTextViewerWithFindReplaceUI("alinee\naLinee\nline\nline");
 		OverlayAccess dialog= getDialog();
-		IFindReplaceTarget target= dialog.getTarget();
+		IFindReplaceTarget target= getFindReplaceTarget();
 		dialog.setFindText("Line");
-		assertThat(dialog.getTarget().getSelectionText(), is("line"));
-		assertEquals(new Point(1,4), dialog.getTarget().getSelection());
+		assertThat(target.getSelectionText(), is("line"));
+		assertEquals(new Point(1, 4), target.getSelection());
 
 		dialog.select(SearchOptions.CASE_SENSITIVE);
-		assertThat(dialog.getTarget().getSelectionText(), is("Line"));
-		assertEquals(new Point(8,4), dialog.getTarget().getSelection());
+		assertThat(target.getSelectionText(), is("Line"));
+		assertEquals(new Point(8, 4), target.getSelection());
 
 		dialog.unselect(SearchOptions.CASE_SENSITIVE);
 		assertEquals(1, (target.getSelection()).x);
@@ -110,7 +110,7 @@ public void testIncrementalSearchUpdatesAfterChangingOptions() {
 		dialog.unselect(SearchOptions.WHOLE_WORD);
 		assertEquals(1, (target.getSelection()).x);
 		assertEquals(4, (target.getSelection()).y);
-		assertThat(dialog.getTarget().getSelectionText(), is("line"));
+		assertThat(target.getSelectionText(), is("line"));
 	}
 
 	@Test
@@ -153,18 +153,19 @@ public void testRememberReplaceExpandState() {
 	@Test
 	public void testSearchBackwardsWithRegEx() {
 		initializeTextViewerWithFindReplaceUI("text text text");
+		IFindReplaceTarget target= getFindReplaceTarget();
 
 		OverlayAccess dialog= getDialog();
 		dialog.select(SearchOptions.REGEX);
 		dialog.setFindText("text"); // with RegEx enabled, there is no incremental search!
 		dialog.pressSearch(true);
-		assertThat(dialog.getTarget().getSelection().y, is(4));
+		assertThat(target.getSelection().y, is(4));
 		dialog.pressSearch(true);
-		assertThat(dialog.getTarget().getSelection().x, is("text ".length()));
+		assertThat(target.getSelection().x, is("text ".length()));
 		dialog.pressSearch(true);
-		assertThat(dialog.getTarget().getSelection().x, is("text text ".length()));
+		assertThat(target.getSelection().x, is("text text ".length()));
 		dialog.pressSearch(false);
-		assertThat(dialog.getTarget().getSelection().x, is("text ".length()));
+		assertThat(target.getSelection().x, is("text ".length()));
 	}
 
 	@Test

bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlay.java

@@ -21,28 +21,27 @@
 import java.util.Objects;
 import java.util.Set;
 import java.util.function.Predicate;
-import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.widgets.Button;
+import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Control;
 import org.eclipse.swt.widgets.Event;
-import org.eclipse.swt.widgets.Shell;
 import org.eclipse.swt.widgets.ToolItem;
 
 import org.eclipse.text.tests.Accessor;
 
 import org.eclipse.jface.text.IFindReplaceTarget;
 import org.eclipse.jface.text.IFindReplaceTargetExtension;
 
-import org.eclipse.ui.internal.findandreplace.FindReplaceLogic;
-import org.eclipse.ui.internal.findandreplace.IFindReplaceLogic;
 import org.eclipse.ui.internal.findandreplace.IFindReplaceUIAccess;
 import org.eclipse.ui.internal.findandreplace.SearchOptions;
 
 class OverlayAccess implements IFindReplaceUIAccess {
-	FindReplaceLogic findReplaceLogic;
+	private final IFindReplaceTarget findReplaceTarget;
+
+	Composite overlayControl;
 
 	HistoryTextWrapper find;
 
@@ -70,11 +69,10 @@ class OverlayAccess implements IFindReplaceUIAccess {
 
 	Accessor dialogAccessor;
 
-	private Supplier<Shell> shellRetriever;
-
-	OverlayAccess(Accessor findReplaceOverlayAccessor) {
+	OverlayAccess(IFindReplaceTarget findReplaceTarget, Accessor findReplaceOverlayAccessor) {
+		this.findReplaceTarget= findReplaceTarget;
 		dialogAccessor= findReplaceOverlayAccessor;
-		findReplaceLogic= (FindReplaceLogic) findReplaceOverlayAccessor.get("findReplaceLogic");
+		overlayControl= (Composite) findReplaceOverlayAccessor.get("overlayControl");
 		find= (HistoryTextWrapper) findReplaceOverlayAccessor.get("searchBar");
 		replace= (HistoryTextWrapper) findReplaceOverlayAccessor.get("replaceBar");
 		caseSensitive= (ToolItem) findReplaceOverlayAccessor.get("caseSensitiveSearchButton");
@@ -87,12 +85,6 @@ class OverlayAccess implements IFindReplaceUIAccess {
 		replaceButton= (ToolItem) findReplaceOverlayAccessor.get("replaceButton");
 		replaceAllButton= (ToolItem) findReplaceOverlayAccessor.get("replaceAllButton");
 		inSelection= (ToolItem) findReplaceOverlayAccessor.get("searchInSelectionButton");
-		shellRetriever= () -> ((Shell) findReplaceOverlayAccessor.invoke("getShell", null));
-	}
-
-	@Override
-	public IFindReplaceTarget getTarget() {
-		return findReplaceLogic.getTarget();
 	}
 
 	private void restoreInitialConfiguration() {
@@ -228,11 +220,6 @@ public void pressSearch(boolean forward) {
 		}
 	}
 
-	@Override
-	public IFindReplaceLogic getFindReplaceLogic() {
-		return findReplaceLogic;
-	}
-
 	@Override
 	public void performReplaceAll() {
 		openReplaceDialog();
@@ -277,25 +264,23 @@ public void assertInitialConfiguration() {
 		assertUnselected(SearchOptions.REGEX);
 		assertUnselected(SearchOptions.WHOLE_WORD);
 		assertUnselected(SearchOptions.CASE_SENSITIVE);
-		if (!doesTextViewerHaveMultiLineSelection(findReplaceLogic.getTarget())) {
+		if (!doesTextViewerHaveMultiLineSelection()) {
 			assertSelected(SearchOptions.GLOBAL);
-			assertTrue(findReplaceLogic.isActive(SearchOptions.GLOBAL));
 		} else {
 			assertUnselected(SearchOptions.GLOBAL);
-			assertFalse(findReplaceLogic.isActive(SearchOptions.GLOBAL));
 		}
 		assertEnabled(SearchOptions.GLOBAL);
 		assertEnabled(SearchOptions.REGEX);
 		assertEnabled(SearchOptions.CASE_SENSITIVE);
-		if (getFindText().equals("") || findReplaceLogic.isAvailable(SearchOptions.WHOLE_WORD)) {
+		if (!getFindText().contains(" ")) {
 			assertEnabled(SearchOptions.WHOLE_WORD);
 		} else {
 			assertDisabled(SearchOptions.WHOLE_WORD);
 		}
 	}
 
-	private boolean doesTextViewerHaveMultiLineSelection(IFindReplaceTarget target) {
-		if (target instanceof IFindReplaceTargetExtension scopeProvider) {
+	private boolean doesTextViewerHaveMultiLineSelection() {
+		if (findReplaceTarget instanceof IFindReplaceTargetExtension scopeProvider) {
 			return scopeProvider.getScope() != null; // null is returned for global scope
 		}
 		return false;
@@ -323,15 +308,19 @@ public void assertEnabled(SearchOptions option) {
 
 	@Override
 	public boolean isShown() {
-		return shellRetriever.get() != null && shellRetriever.get().isVisible();
+		return overlayControl.isVisible();
 	}
 
 	@Override
 	public boolean hasFocus() {
-		Shell overlayShell= shellRetriever.get();
-		Control focusControl= overlayShell.getDisplay().getFocusControl();
-		Shell focusControlShell= focusControl != null ? focusControl.getShell() : null;
-		return focusControlShell == overlayShell;
+		Control focusControl= overlayControl.getDisplay().getFocusControl();
+		while (focusControl != null) {
+			if (overlayControl == focusControl) {
+				return true;
+			}
+			focusControl= focusControl.getParent();
+		}
+		return false;
 	}
 
 }