Bug 317616

Comment 1 Rüdiger Herrmann 2010-06-22 12:53:46 EDT

Created attachment 172438 [details]
Snippet to reproduce

Comment 2 Ivan Furnadjiev 2010-07-09 08:41:57 EDT

This regression was introduced with the fix for bug 301261. In case of 'synchronous' key listeners we have two 'standalone' requests - the UI callback and the request from the key events. With the fix for bug 301261 we dispose the previous request before creating a new one (in Request#_sendStandalone), but in this case we have 2 independent requests and it is not correct to dispose the running UICallBack request with the creation of key event request.

Comment 3 Ivan Furnadjiev 2010-07-12 06:59:09 EDT

Created attachment 174011 [details]
Proposed patch

This patch removes the previous fix for bug 301261 and dispose only finished (completed or failed ) requests in the corresponding event handling method - _handleCompleted and _handleFailed. Ruediger, what do you think about such a fix?

Comment 4 Ralf Sternberg 2010-07-15 12:22:34 EDT

Reviewed the patch together with Tim. The patch solves the problem. In the case of async requests, the exchange object is now disposed unnecessarily (the qx request queue also does this), but that doesn't hurt. This can be refactored at a suitable occasion in HEAD.

Applied patch to CVS HEAD and 1.3 maintenance branch.

Comment 5 lyp 2010-08-03 05:09:40 EDT

Hi all experts,
When disabling Session Cookies in Jetty 6.x with the configuration below, I found that it disable tabbed browsing to run RAP applications on multiple browser-tabs In IE7 using this patch. But enabled to run on In IE6 and FireFox.

I am documenting the configuration below. Create the file jetty-web.xml and place it in the WEB-INF/ directory. Populate that file with the following contents.

=============================================================================
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE Configure PUBLIC "-//Mort Bay Consulting//DTD Configure//EN"
    "http://jetty.mortbay.org/configure.dtd">
<Configure class="org.mortbay.jetty.webapp.WebAppContext">
    <Get name="sessionHandler">
        <Get name="sessionManager">
            <Set name="usingCookies" type="boolean">false</Set>
        </Get>
    </Get>
</Configure>
=============================================================================

However, I try to delete the patch code When disabling Session Cookies in Jetty 6.x, And It can work fine to enable tabbed browsing In IE7 when I delete the patch code below:

=============================================================================
    _handleFailed : function( evt ) {

        var request = exchange.getImplementation().getRequest();

      // [if] Dispose the only finished transport - see bug 301261, 317616
			exchange.dispose();
    },


    _handleCompleted : function( evt ) {
    	var exchange = evt.getTarget();
      var text = exchange.getImplementation().getRequest().responseText;
      // [if] Dispose the only finished transport - see bug 301261, 317616
      exchange.dispose();
    },
=============================================================================

So, why the code upper disable tabbed browsing RAP In IE7 When disabling Session Cookies in Jetty 6.x.

Comment 6 Ralf Sternberg 2010-08-03 05:43:53 EDT

(In reply to comment #5)
I don't quite understand. Can you give a step-by-step instruction how to reproduce your problem? What exactly is the behavior you see and what would you expect instead?

If you think this fix introduces a problem, please reopen this bug.

Comment 7 lyp 2010-08-03 07:05:25 EDT

hi Ralf,
I can reproduce the tabbed browsing problem by the following steps.

step 1: Disabling Session Cookies in Jetty 6.x with the configuration below:
=============================================================================
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE Configure PUBLIC "-//Mort Bay Consulting//DTD Configure//EN"
    "http://jetty.mortbay.org/configure.dtd">
<Configure class="org.mortbay.jetty.webapp.WebAppContext">
    <Get name="sessionHandler">
        <Get name="sessionManager">
            <Set name="usingCookies" type="boolean">false</Set>
        </Get>
    </Get>
</Configure>
=============================================================================

step2:  Run the RAP application with the patch code:
=============================================================================
    _handleFailed : function( evt ) {

        var request = exchange.getImplementation().getRequest();

      // [if] Dispose the only finished transport - see bug 301261, 317616
            exchange.dispose();
    },


    _handleCompleted : function( evt ) {
        var exchange = evt.getTarget();
      var text = exchange.getImplementation().getRequest().responseText;
      // [if] Dispose the only finished transport - see bug 301261, 317616
      exchange.dispose();
    },
=============================================================================

step3: Open RAP applications on multiple browser-tabs In IE7. Then the problem is reproduced. The RAP application is loading always on multiple browser-tabs, and the RAP applications have not any response on all opened tabs.

step 4: I try to delete the patch code, And also Open RAP applications on multiple browser-tabs In IE7, Then the problem is solved. The rap UI can work fine on multiple browser-tabs In IE7.

Comment 8 Michael Fröschen 2010-09-14 10:22:21 EDT

Is that only fixed on 1.4 M1 or is it supposed to be fixed in 1.3.1 too?

Comment 9 Ivan Furnadjiev 2010-09-15 03:02:24 EDT

Yes. It's fixed in 1.3.1 too.

Comment 10 Austin Riddle 2010-10-07 18:04:20 EDT

We have been experiencing a return of UI callback problems since the patch that was applied in this bug, particularly related to progress dialogs.

Our progress dialogs were hanging for a long time before finally getting dismissed.  And then our apps would behave like the UI Callback was dead.

After looking at the patch in this bug, which removed the patch from bug #301261, we determined that from our vantage point the UI Callback transport was still being leaked because the dispose calls on the exchange were in listeners that were not in the same control path as enableUICallback().  So we re-applied the patch from that bug in enableUICallback() to fix the problems we were having.

In other words, the listeners that would dispose the transport were not being registered for enableUICallback() because it did not use createRequest() and simply called _sendStandalone().  

It all seems to be working fine now.  This may not be the ultimate fix, but I do think this addresses the problem.

Comment 11 Austin Riddle 2010-10-07 18:04:44 EDT

Created attachment 180459 [details]
Patch to handle separate control path

Comment 12 Ivan Furnadjiev 2010-10-08 07:57:44 EDT

Austin, as this bug is still fixed (KeyListener does not break UICallback anymore) I will close it. Let continue the discussion on this bug 301261.