Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 364034

Summary: [QuickAccess] Does not close or reposition when parent window moved
Product: [Eclipse Project] Platform Reporter: Dean Roberts <dean.t.roberts>
Component: UIAssignee: Dean Roberts <dean.t.roberts>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: remy.suen
Version: 4.2Flags: dean.t.roberts: review? (remy.suen)
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
Patch to close drop down win window moved or resized
none
Patch attempt 2
none
Patch without asyncExec none

Description Dean Roberts CLA 2011-11-17 09:16:34 EST 
1) Open Quick Access
2) Move the Workbench window around

The Quick Access table neither closes nor moves to maintain its position relative to the workbench window.

Given that the new SearchField is supposed to behave like a drop down, it should do one or the other.

Does anybody have an opinion on the desired behaviour?
Comment 1 Remy Suen CLA 2011-11-21 09:05:28 EST 
(In reply to comment #0)
> Does anybody have an opinion on the desired behaviour?

I'd say it should be closed.
Comment 2 Dean Roberts CLA 2011-11-29 09:21:07 EST 
Created attachment 207652 [details]
Patch to close drop down win window moved or resized
Comment 3 Remy Suen CLA 2011-11-30 12:54:19 EST 
Comment on attachment 207652 [details]
Patch to close drop down win window moved or resized

(In reply to comment #2)
> Created attachment 207652 [details]
> Patch to close drop down win window moved or resized

1. Open a second workbench window.
2. Ctrl+3
3. Type some stuff.
4. Close the second workbench window while the QA shell is up.
5. An SWTException will be thrown. May need to try a few times.

Caused by: org.eclipse.swt.SWTException: Widget is disposed
	at org.eclipse.swt.SWT.error(SWT.java:4300)
	at org.eclipse.swt.SWT.error(SWT.java:4215)
	at org.eclipse.swt.SWT.error(SWT.java:4186)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:468)
	at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:340)
	at org.eclipse.swt.widgets.Text.getText(Text.java:1114)
	at org.eclipse.ui.internal.quickaccess.SearchField$1$1.run(SearchField.java:114)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:135)
	... 24 more
Comment 4 Dean Roberts CLA 2011-11-30 13:42:24 EST 
Created attachment 207751 [details]
Patch attempt 2

After many tries I could not reproduce the exception.

I've added a new patch that checks both the composite and the text widget are not disposed.
Comment 5 Remy Suen CLA 2011-11-30 14:06:54 EST 
(In reply to comment #4)
> Created attachment 207751 [details]
> Patch attempt 2

I'm wondering if it makes sense to have this runnable be queued every time the workbench window moves or is resized. It seems like a lot of runnables would get queued if you're just moving your window around.

What is the reason behind making this call asynchronous?
Comment 6 Dean Roberts CLA 2011-12-01 13:39:38 EST 
Created attachment 207804 [details]
Patch without asyncExec

Talk to Bogdan and he agreed that, in this case, there is no need for the close code to be in an asyncExec call.

I originally did it because I was modeling it after the checkFocusLost() code.  I believe the asyncExec is required for that code because the setText() call will cause a focusChange for the SearchField shell.  This is not the case for this resize/move case.
Comment 7 Dean Roberts CLA 2012-02-13 14:17:22 EST 
Embarrassingly this fix "snuck" out in a completely unrelated patch 9 weeks ago.

It was a large patch having to do with getting getting the E4 tests to compile.

The commit was:

http://git.eclipse.org/c/platform/eclipse.platform.ui.git/diff/bundles/org.eclipse.ui.workbench/Eclipse%20UI/org/eclipse/ui/internal/quickaccess/SearchField.java?id=412bb792f91ba461ddb4451860a72ff58cf8a279