Shindig
  1. Shindig
  2. SHINDIG-1497

pubsub2 breaks due to exception after drag and drop of gadget

    Details

    • Type: Bug Bug
    • Status: Patch Available
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.0.2, 2.5.0-beta1
    • Fix Version/s: None
    • Component/s: Javascript
    • Labels:
      None

      Description

      After dragging and dropping a gadget rendered using pubsub-2, the following exception is thrown when attempting to pub/sub/unsubscribe:

      "Component returned failure code: 0xc1f30001 (NS_ERROR_NOT_INITIALIZED) nsIDOMJSWindow.setTimeout"

      The issue appears to be in rpc.js in the method callSameDomain(target, rpc) (line 481 in rev 1065708). The first "if" clause is an optimization to avoid redefining sameDomain[target] (line 495) if the value is already defined. But the problem is that after you drag and drop, the target element value targetEl (line 492) has changed, which also means the value of sameDomain[target] has changed (line 495). But since sameDomain[target] is already defined, the code doesn't update the value of sameDomain[target], but instead jumps to calling the rpc (line 505).

      A simple workaround would be to comment out the "if" clause and matching brace (lines 482 and 501), but this of course removes the optimization as well. A better approach may be to have a callback to update sameDomain[target] or a global boolean variable indicating when sameDomain[target] needs to be updated.

        Activity

        Dennis Ju created issue -
        Hide
        Dennis Ju added a comment -

        simple workaround

        Show
        Dennis Ju added a comment - simple workaround
        Dennis Ju made changes -
        Field Original Value New Value
        Attachment SHINDIG-1497.diff [ 12469845 ]
        Dennis Ju made changes -
        Description After dragging and dropping a gadget rendered using pubsub-2, the following exception is thrown when attempting to pub/sub/unsubscribe:

        "Component returned failure code: 0xc1f30001 (NS_ERROR_NOT_INITIALIZED) nsIDOMJSWindow.setTimeout"

        The issue appears to be in rpc.js in the method callSameDomain(target, rpc). The first "if" clause is an optimization to avoid redefining sameDomain[target] (line 495) if the value is already defined. But the problem is that after you drag and drop, the target element value targetEl (line 492) has changed, which also means the value of sameDomain[target] has changed (line 495). But since sameDomain[target] is already defined, the code doesn't update the value of sameDomain[target], but instead jumps to calling the rpc (line 505).

        A simple workaround would be to comment out the "if" clause and matching brace (lines 482 and 501), but this of course removes the optimization as well. A better approach may be to have a callback to update sameDomain[target] or a global boolean variable indicating when sameDomain[target] needs to be updated.

        After dragging and dropping a gadget rendered using pubsub-2, the following exception is thrown when attempting to pub/sub/unsubscribe:

        "Component returned failure code: 0xc1f30001 (NS_ERROR_NOT_INITIALIZED) nsIDOMJSWindow.setTimeout"

        The issue appears to be in rpc.js in the method callSameDomain(target, rpc) (line 481 in rev 1065708). The first "if" clause is an optimization to avoid redefining sameDomain[target] (line 495) if the value is already defined. But the problem is that after you drag and drop, the target element value targetEl (line 492) has changed, which also means the value of sameDomain[target] has changed (line 495). But since sameDomain[target] is already defined, the code doesn't update the value of sameDomain[target], but instead jumps to calling the rpc (line 505).

        A simple workaround would be to comment out the "if" clause and matching brace (lines 482 and 501), but this of course removes the optimization as well. A better approach may be to have a callback to update sameDomain[target] or a global boolean variable indicating when sameDomain[target] needs to be updated.

        Hide
        Maxwell Chiareli added a comment -

        What does change when drag and drop? maybe it's possible to add another condition to if clause, like: if (typeof sameDomain[target] === 'undefined' || sameDomain[target].someProperty != target.someProperty) . I did not find any different property

        Show
        Maxwell Chiareli added a comment - What does change when drag and drop? maybe it's possible to add another condition to if clause, like: if (typeof sameDomain [target] === 'undefined' || sameDomain [target] .someProperty != target.someProperty) . I did not find any different property
        Hide
        Maxwell Chiareli added a comment -

        the patch did not work for me, after apply i got the error:

        Same domain call failed: parent= incorrectly set.

        Show
        Maxwell Chiareli added a comment - the patch did not work for me, after apply i got the error: Same domain call failed: parent= incorrectly set.
        Hide
        Maxwell Chiareli added a comment -

        also had to change the getTargetWin method and remove the code:

        var target = window.frames[id];
        if (target)

        { return target; }

        Seems like the drag/drop action, remove some properties from the iframe, like location, so you always you get a error with the current validation.

        Show
        Maxwell Chiareli added a comment - also had to change the getTargetWin method and remove the code: var target = window.frames [id] ; if (target) { return target; } Seems like the drag/drop action, remove some properties from the iframe, like location, so you always you get a error with the current validation.
        Hide
        Dennis Ju added a comment -

        Upon re-testing, I did indeed have to change getTargetWin as well. Not sure why it worked for me the first time.

        Any input from the community on the appropriateness of this workaround/fix?

        Show
        Dennis Ju added a comment - Upon re-testing, I did indeed have to change getTargetWin as well. Not sure why it worked for me the first time. Any input from the community on the appropriateness of this workaround/fix?
        Gavin made changes -
        Workflow jira [ 12544146 ] patch-available, re-open possible [ 12630035 ]
        Hide
        Ilya Shtein added a comment -

        These two changes fix the original exception(s) with Drag-and-Drop. However, they still do not fix Drag-and-Drop for pubsub-2 gadgets:
        Upon dropping a pubsub-2 gadget,the gadget will get a new subscription every time it is dropped, whereas the old one is never canceled (unsubscribed).
        E.g., if my gadget invokes gadgets.Hub.subscribe once for message “aaa.bbb.ccc” when created, it will originally receive one such message each time the message is published. After dragging and dropping the gadget, it will receive two such messages, after second drag-and-drop – three, etc.
        Apparently, when an IFrame (gadget) is moved in the DOM tree, no unsubscribe is called for its existing subscriptions; the IframeContainer code is written in such way that every attempt to subscribe to the same message grows the number of active subscriptions the Hub, no matter whether a subscription for a given message exists or not.
        I am not allowed to contribute directly to the Open Source community, but here is my fix for this latter problem... can someone please take it and implement as a patch?

        • File iframe.js, class OpenAjax.hub.IframeContainer, method this._handleIncomingRPC, beginning at line 298 (I am using OpenAjax 2.0.5):
          ************************************
          case "sub":
          var errCode = ""; // empty string is success
          try {
          /* Begin new code */
          if( subs[data] ) { // If a client subscription with // this ID already exists, delete it // This will ensure uniqueness // of the client subscription // when the client is regenerated // during drag-and-drop hub.unsubscribeForClient(container, subs[data]); delete subs[data]; }

          /* End new code */

        subs[ data ] = hub.subscribeForClient( container, topic, data );
        } catch( e )

        { errCode = e.message; }

        return errCode;
        ****************************************************
        (The beginning and the end of the new code are marked inside the snippet).
        In my tests, this fixed the multiple subscriptions problem by ensuring that a gadget may only subscribe once for any given message (which I think makes sense).
        As for the original Dennis's fix, I don't see any feedback from the community to the question posed by Dennis about the appropriateness of such fix. But I can attest that the fixes from Dennis and Maxwell, as well as the fix I am suggesting, fix the drag-and-drop problem for pubsub-2 gadgets.

        Show
        Ilya Shtein added a comment - These two changes fix the original exception(s) with Drag-and-Drop. However, they still do not fix Drag-and-Drop for pubsub-2 gadgets: Upon dropping a pubsub-2 gadget,the gadget will get a new subscription every time it is dropped, whereas the old one is never canceled (unsubscribed). E.g., if my gadget invokes gadgets.Hub.subscribe once for message “aaa.bbb.ccc” when created, it will originally receive one such message each time the message is published. After dragging and dropping the gadget, it will receive two such messages, after second drag-and-drop – three, etc. Apparently, when an IFrame (gadget) is moved in the DOM tree, no unsubscribe is called for its existing subscriptions; the IframeContainer code is written in such way that every attempt to subscribe to the same message grows the number of active subscriptions the Hub, no matter whether a subscription for a given message exists or not. I am not allowed to contribute directly to the Open Source community, but here is my fix for this latter problem... can someone please take it and implement as a patch? File iframe.js, class OpenAjax.hub.IframeContainer, method this._handleIncomingRPC, beginning at line 298 (I am using OpenAjax 2.0.5): ************************************ case "sub": var errCode = ""; // empty string is success try { /* Begin new code */ if( subs [data] ) { // If a client subscription with // this ID already exists, delete it // This will ensure uniqueness // of the client subscription // when the client is regenerated // during drag-and-drop hub.unsubscribeForClient(container, subs[data]); delete subs[data]; } /* End new code */ subs[ data ] = hub.subscribeForClient( container, topic, data ); } catch( e ) { errCode = e.message; } return errCode; **************************************************** (The beginning and the end of the new code are marked inside the snippet). In my tests, this fixed the multiple subscriptions problem by ensuring that a gadget may only subscribe once for any given message (which I think makes sense). As for the original Dennis's fix, I don't see any feedback from the community to the question posed by Dennis about the appropriateness of such fix. But I can attest that the fixes from Dennis and Maxwell, as well as the fix I am suggesting, fix the drag-and-drop problem for pubsub-2 gadgets.
        Hide
        Ilya Shtein added a comment -
        • File iframe.js, class OpenAjax.hub.IframeContainer, method this._handleIncomingRPC, beginning at line 298 (I am using OpenAjax 2.0.5):
                                                                              • case "sub":
                                                                                var errCode = ""; // empty string is success
                                                                                try {
                                                                                /* Begin new code */
                                                                                if( subs[data] )

                                                                                { // If a client subscription with this ID already exists, delete it // This will ensure uniqueness of the client subscription // when the client is regenerated during drag-and-drop hub.unsubscribeForClient(container, subs[data]); delete subs[data]; }


                                                                                /* End new code */

        subs[ data ] = hub.subscribeForClient( container, topic, data );
        } catch( e )

        { errCode = e.message; }


        return errCode;

                                                                                                              • (The beginning and the end of the new code are marked inside the snippet).

        Show
        Ilya Shtein added a comment - File iframe.js, class OpenAjax.hub.IframeContainer, method this._handleIncomingRPC, beginning at line 298 (I am using OpenAjax 2.0.5): case "sub": var errCode = ""; // empty string is success try { /* Begin new code */ if( subs [data] ) { // If a client subscription with this ID already exists, delete it // This will ensure uniqueness of the client subscription // when the client is regenerated during drag-and-drop hub.unsubscribeForClient(container, subs[data]); delete subs[data]; } /* End new code */ subs[ data ] = hub.subscribeForClient( container, topic, data ); } catch( e ) { errCode = e.message; } return errCode; (The beginning and the end of the new code are marked inside the snippet).
        Ilya Shtein made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Patch Available Patch Available
        359d 20m 1 Ilya Shtein 25/Jan/12 19:25

          People

          • Assignee:
            Unassigned
            Reporter:
            Dennis Ju
          • Votes:
            2 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development