Community
Participate
Working Groups
I20080513-0817 I opened the Available Software page and typed "ECF". The dialog disappeared! At first I thought there was some horrible regression introduced by my fix to bug 226052. But I realized that instead, depending on how fast you type, the "C" in ECF was being interpreted as the "C" mnemonic for the Close button. When we disable the filter text to show that we are starting a load of the repos, the focus goes (somewhere?) and that widget is interpreting the keystroke as a button mnemonic. We need to put the focus somewhere safe while the load is going on.
marking RC1 but it can probably get pushed to RC2 if we run out of time.
moving all bugs that didn't make RC1 to RC2. Will triage those and determine when/what will get fixed during this time.
Is disabling the filter text where you want to go? When controls go grey without me noticing I'm generally ticked off, if I notice at all. Could we not instead simply disable the tree control and leave the filter alive?
>Is disabling the filter text where you want to go? No, it's really not where I want to be, but it's where I am. An RC1 stopgap in bug #226052. Lots of explanation there, but in a nutshell: The tree uses deferred content fetching, so that sites are retrieved as you expand them. Similar to CVS repos. This works nicely until you want to filter. Filter refreshes viewer and filters on content, you end up filtering on "Pending..." not what user wants. The first keystroke means the whole tree needs to be expanded, and we don't want to filter until the real content is there. Often this is a time sink because our loading can be quite slow depending on nature of sites (metadata generation happening, downloading of artifacts, etc.). We don't want to preload everything until we know you really want to do that...So for now we trigger a load and set the content provider to a synchronous mode so that filtering will behave properly once content is loaded. We could do what you suggest by having our filtered tree subclass not trigger filtering until the load ends, but we ended up disabling most of the dialog out of conservatism this late in the release since anyone else going for a repo could hang the UI thread.
I believe Eric has been poking around here...
yep, he pinged me...just answering your question. I'm going to move your specific suggestion to a new bug to be considered later. I plan to revisit this whole area next release (either through data binding or having an implementation of fetch/filter/checkboxes specific to this situation.) Right now we have a house of bandaids trying to make the platform UI components work together in this area.
Created attachment 101324 [details] Patch that diables the close button on the Dialog (but not the red 'X') This patch does two things; - moves the global dis-ablement up to the shell level to ensure the close button is disabled (and then re-enables the shell to get the red 'X' back) - changes the order of the re-enablement to enable the controls before trying to reset the focus (SWT won't allow focus to a disabled control). Susan, take a look and let me know what you think...
thanks, Eric, for the help. I'll review first and find a second reviewer.
Hmmm...I'm seeing some strange behavior with the patch. If I type "EC" where the C hits after the control is disabled, this seems to cause the UI thread to lock up briefly. Or at least, I see the progress animation stop, nothing is accessible, then it eventually unlocks. I compared this to the behavior without the patch (using a letter that won't close the dialog) and without the patch there is no perceivable lockup. I thought that it might be a problem with disabling and reenabling the shell (that would cause shell activate/deactivate events), so I tried a hack that disabled the shell's child (knowing it is the dialog area) and that had the same problem. Another hack that disabled the button bar in addition to the control we previously disabled also caused the problem. I thought it might be coincidence (ie, maybe it's a GC or something else that happens occasionally) but I can consistently make it happen with the patch and it hasn't happened to me without the patch. I'll take a look at this again on the latest build. I am running from HEAD with some new stuff from p2 that could change the load sequence, and with some patches that are awaiting review, but in comparing this patch against no patch, I'm running the same code in the workspace besides the patch.
Confirmed behavior on latest build. The lockup *does not* happen without the patch. The other oddity is that with the patch (or my alternatives), the tree expansion starts showing (dancing tree) in the disabled tree as soon as the test key is hit, whereas it doesn't show without the patch. Debugging shows I'm not getting any unexpected p2 events that would refresh the viewer from the UI thread. The load job completion is also happening normally - something else tries to hit a repo from the UI thread when that key is hit. I switched gears, trying to solve on the client end by ignoring mnemonic traversal events when the filtering is disabled. This is a different brand of hack, but at least it's very specific to the bug and doesn't change the UI event flow. Attaching a patch.
Created attachment 101642 [details] different strategy this patch ignores mnemonic traversal events in the dialog when the filter text is disabled.
John and DJ - can you please review? Summarizing problem statement to save you reading about previous failures ;-) Problem: - launch Eclipse with a bunch of repos in your repo list. You need to start fresh, knowing repos aren't loaded yet. - open "Available Software" and type "E" to start filtering. - When the filter text goes gray, type "C" - Dialog closes (bad). The widget with the focus interpreted "C" as a mnemonic traversal event and closed the dialog. Solution: - we tried several alternatives but they introduced change in event flow that hung up the UI thread during load. - the least impacting solution is to hook traversal events on the button and its parents. If the event is a mnenomic traversal while the filter text is disabled, ignore it. Things to test: - launch Eclipse with a bunch of repos in your repo list. You need to start fresh, knowing repos aren't loaded yet. - open "Available Software" and type a letter to start filtering - try these keys below to make sure it doesn't close when you don't mean it to - you may need to close/restart several times to hit all the cases Verify that: - typing C doesn't close the dialog or hang the UI - you can still type Esc to close the dialog - you can still type Alt+C to close the dialog - you can still use the other Alt+ mnemonics - you can hit the close button with the mouse - you can hit Esc to close I verified all this on windows. I will make sure people try this on Mac and Linux during RC2 test pass.
DJ & John - I forgot to mention that there is a bonus fix in the patch, found while testing. In the aboutToRun() listener on the filter job, we check the filter text in a sync exec, but do not do the requisite dispose check. (See what's done in the later async exec). Normally you don't have to worry about this in a sync exec because it's running synchronously, but since the job event itself can be triggered after the dialog is gone, the dispose state needs to be checked when handling the event. While trying to close the widget with various keystrokes during filtering, I occasionally got the SWT not disposed error because of this.
Created attachment 101667 [details] patch +1 on the patch with a minor change that I discussed with Susan and made. (fix up boolean logic in an if statement)
+1 on revision. Yes, the syncExec check should match the asyncExec check underneath.
Released.
