Find/replace overlay: replace shell with integrated composite eclipse…

…-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

bundles/org.eclipse.ui.workbench.texteditor/src/org/eclipse/ui/texteditor/FindReplaceAction.java

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

tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/FindReplaceUITest.java

@@ -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();
+ }
+
 }

tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/IFindReplaceUIAccess.java

@@ -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();

tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/FindReplaceOverlayTest.java

@@ -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

tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/internal/findandreplace/overlay/OverlayAccess.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;
  }
 
 }

tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/DialogAccess.java

@@ -35,7 +35,6 @@
 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;
 
@@ -199,11 +198,6 @@ public void setReplaceText(String text) {
  replaceCombo.notifyListeners(SWT.Modify, null);
  }
 
- @Override
- public IFindReplaceTarget getTarget() {
- return findReplaceLogic.getTarget();
- }
-
  @Override
  public String getFindText() {
  return findCombo.getText();
@@ -219,11 +213,6 @@ public Combo getFindCombo() {
  return findCombo;
  }
 
- @Override
- public IFindReplaceLogic getFindReplaceLogic() {
- return findReplaceLogic;
- }
-
  @Override
  public void performReplaceAll() {
  replaceAllButton.notifyListeners(SWT.Selection, null);

tests/org.eclipse.ui.workbench.texteditor.tests/src/org/eclipse/ui/workbench/texteditor/tests/FindReplaceDialogTest.java

@@ -122,7 +122,7 @@ public void testShiftEnterReversesSearchDirectionDialogSpecific() {
 
  dialog.setFindText("line");
  ensureHasFocusOnGTK();
- IFindReplaceTarget target= dialog.getTarget();
+ IFindReplaceTarget target= getFindReplaceTarget();
 
  dialog.simulateKeyboardInteractionInFindInputField(SWT.CR, false);
  assertEquals(0, (target.getSelection()).x);
@@ -152,7 +152,7 @@ public void testReplaceAndFindAfterInitializingFindWithSelectedString() {
 
  assertThat(dialog.getFindText(), is("text"));
 
- IFindReplaceTarget target= dialog.getTarget();
+ IFindReplaceTarget target= getFindReplaceTarget();
  assertEquals(0, (target.getSelection()).x);
  assertEquals(4, (target.getSelection()).y);
 
@@ -209,7 +209,7 @@ public void testFindWithWholeWordEnabledWithMultipleWordsNotIncremental() {
  dialog.setFindText("two");
  dialog.select(SearchOptions.WHOLE_WORD);
  dialog.select(SearchOptions.WRAP);
- IFindReplaceTarget target= dialog.getTarget();
+ IFindReplaceTarget target= getFindReplaceTarget();
 
  dialog.simulateKeyboardInteractionInFindInputField(SWT.CR, false);
  assertEquals(0, (target.getSelection()).x);