Bugzilla – Full Text Bug Listing
| Summary: | [Shell] Widget is disposed exception is thrown in a somewhat peculiar configuration | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [RT] RAP | Reporter: | Istvan Ballok <Istvan.Ballok> | ||||||||
| Component: | RWT | Assignee: | Project Inbox <rap-inbox> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | Istvan.Ballok, tbuschto | ||||||||
| Version: | 1.4 | ||||||||||
| Target Milestone: | 1.5 M7 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Windows 7 | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
Created attachment 189213 [details]
patch to reproduce the problem
Further thoughts
================
After some further considerations, I think this behaviour is a bug,
and unfortunately its root is deeper. I can not propose a possible
solution.
If we compare the events, sent from the client, in the 2 cases:
A) 2 slow clicks on the button B) a fast, double click, we can
observe the following difference:
A) 2 slow clicks
1) org.eclipse.swt.events.widgetSelected=w49 -> the first click on the button
2) org.eclipse.swt.events.shellActivated=w2 -> the second click on the button
first activates the shell that contains it.
(the newly opened shell was active, and hence the main shell containing the
button being clicked, had to be activated before)
3) org.eclipse.swt.events.widgetSelected=w49 -> the second click on the button
OK.
B) a double click
1) org.eclipse.swt.events.widgetSelected=w49 -> the first click on the button
2) org.eclipse.swt.events.widgetSelected=w49 -> the second click on the button
-> widget is disposed exception.
i.e., in the double click case, the shell activation event is not sent.
In fact, the events of B) are not valid!, and that is the root of the problem:
The button should not send a click (selection) event, if it is not in focus,
its containing shell is not activated! The shell activation event must be sent
before the selection event!
How can this happen?
--------------------
The second widget selected event happened (!)before the response of the first
request arrived. And then, the main shell was still active -> there was no need,
to send a shell activated event. Because the first request was pending,
the second event was enqueued, and it was sent later, when the first response arrived.
But, the problem is that due to the first response, this enqueued event is
no longer valid! The main shell has lost its focus due to the first
response; and should be reactivated, before the click event.
General problem
---------------
I think this is a general problem, that can happen, when a second event
happens before the response of the first event arrives. The response can
make the enqueued second event invalid.
Double clicks, or touch displays can cause events at such a frequency,
that there is a real chance for a second event to happen, before the short
time (<50-100 ms) it takes for the first response to arrive.
I can reproduce it too with your snippet. Created attachment 191690 [details]
Same patch (snippet) against CVS HEAD
To summarize the issue of the missing activation event: - mousedown -> nothing happens (main shell already active) - mouseup -> send widgetSelected to server -> start request ( request running ) - mousedown -> shell still active, *therefore no activation* ( request running ) - mouseup -> widgetSelected queued (since request is running) - request finished -> create new shell and activate it, deactivate main shell - send queued widgetSelected request (and no activation event for main shell) --> server gets a widgetSelected-event of a button on a shell thats not activated, activation event gets "lost", because there was none becaue the shell was still active at the time One possible, but very heavy-weight solution could be to re-create the active/focus state before sending a queued request. For this scenario it would cause an activation-event to be added to the second request; However there still is the issue of the order of processing these two events in the same request (activation and selection) on the server. The implementation could look like this:
- create "saveUiState" and "restoreUiState" functions in Request.js
- in Request#_handleCompleted add a call to "saveUiState" at the beginning
- in same function add after the try/catch block:
if( this._runningRequestCount > 0 ) {
this._restoreUiState();
}
Note that there would probably need to be some checks in "restoreUiState", for example whether the shell/widget in question haven been disposed by the request.
This is all just a theory i have not tested this solution.
When the second (queued) widgetSelected comes to the server the button shell is no longer the active shell. We should no fire events on the controls that are not on the active shell. Maybe the proper solution will be to change the EventUtil#isAccessible *not* to allow events on inactive shells. But in this case it's possible to run in client-server out-of-sync - for example toggle button. Toggle button changes its state on the client, but rejected on the server as the toggle button shell is no longer active. Created attachment 191752 [details] Draft of the solution outlined in comment #7 Out-of-sync problem is not fixed with this patch. The out-of-sync problem could be solved in a generic manner when we have the protocol, just restore the state if the event is thrown away. Currently this can't be done because selection-synchronisation is implemented differently for every widget. See also Bug 223852 which describes the same out-of-sync scenario that we could have when using ivans patch with a toggle-button. The situation described in this bug is related to the application model of RAP. It's one of the few scenarios where asynchronous processing and network latency interfere with the normal flow of events. I doubt that there is a clean and simple way to completely eliminate this kind of problems. However, if this patch helps to work around a specific issue, I think we should integrate it. Currently, the patch does not contain any test case and it even breaks the ActivateEvent_Test. We should also add a comment that points to this bug and makes clear that this change is a workaround. With the fix for Bug 325756 I can't reproduce the issue anymore. Please reopen the bug if the problem persists with the current CVS HEAD or upcoming RAP 1.5M7. |
To reproduce ============ Apply the attached patch (note: it is based on the current git master). This slightly modifies the ShellTab example of the controls demo. Select the "Shell" tab, and perform a double click on the "Open Shell" button. A Widget is disposed exception is thrown. The peculiar configuration ========================== - the button opens a new shell - the shell is accessed when it is activated - the shell is closed when it is deactivated - the button is clicked with a double click* This is the essence of the problem I observed in our application. (*: double click: e.g. instead of a single click, the user performs a double click by mistake) My investigation ================ - the SWT (and qooxdoo) buttons does not support a double click - a double click is conveyed as too seperate click events, one after the other - the second event is queued in the client, and hence it is not the same as 2 slow clicks. (see focus handling) # the first click opens a shell, it is focused. # the second click event arrives # (the focus is not changed, because it is the old, queued event from the client) # the second shell is opened: during open, # first, the other shells are deactivated -> the first shell loses focus / is deactivated -> the first shell is closed -> during its disposition, an (!)ancestor of the first shell receives the focus -> (!)the second shell is deactivated, as a consequence -> the second shell is disposed # second, the new shell is activated -> the second shell is activated - but it is already disposed Is it a bug in RWT? =================== I am not sure, I have not yet tested this snippet with SWT. But even if it is not considered a bug, it is certainly an unexpected behaviour that is worth sharing/discussing in a bugzilla issue.