1. #1
    Ext JS Premium Member
    Join Date
    Mar 2008
    Location
    Germany, Frankfurt
    Posts
    142
    Vote Rating
    1
    enpasos is on a distinguished road

      0  

    Default [4.1 RC3]: DragDropManager.fireEvents - wrong parameters calling onInvalidDrop

    [4.1 RC3]: DragDropManager.fireEvents - wrong parameters calling onInvalidDrop


    Within the method DragDropManager.fireEvents the method onInvalidDrop on the DragSource is called with the wrong parameters. See the following debug screenshot

    dd_bug.jpg

    Instead of entering (target, event, id) it is called with (event).

  2. #2
    Sencha Premium Member skirtle's Avatar
    Join Date
    Oct 2010
    Location
    UK
    Posts
    3,590
    Vote Rating
    322
    skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future

      0  

    Default


    I agree something looks amiss here but it isn't immediately clear to me whether it's DragDropManager or DragSource that's doing it wrong. It also doesn't look like any of that code has really changed since 4.0.

    Do you happen to have a simple test case that exhibits a failure resulting from this problem?

  3. #3
    Ext JS Premium Member
    Join Date
    Mar 2008
    Location
    Germany, Frankfurt
    Posts
    142
    Vote Rating
    1
    enpasos is on a distinguished road

      0  

    Default


    In line 752 in DragDropManager a method is called with the single parameter of type Event.

    Code:
            // notify about a drop that did not find a target
            if (isDrop && !dropEvts.length) {
                dragCurrent.onInvalidDrop(e);
            }
    "dragCurrent" is of type 'Ext.dd.DragDrop' and the method is declared there as
    Code:
      /**
         * Abstract method called when this item is dropped on an area with no
         * drop target
         * @param {Event} e the mouseup event
         */
        onInvalidDrop: function(e) { /* override this */ },
    So far it is OK. 'Ext.dd.DragDrop' is extended by 'Ext.dd.DD' extended by 'Ext.dd.Proxy' extended by 'Ext.dd.DragSource'. Now comes the Bug 'Ext.dd.DragSource' defines a method with different parameters:
    Code:
        // private
        onInvalidDrop: function(target, e, id) {
    Therefore the call of DragDropManager in line 752 relies on an interface declaration that is not fulfilled.

    To give an example of subsequent problems that may rise from this: If the on InvalidDrop method in 'Ext.dd.DD' is called with (e) instead of (target, e, id) then the call in line 266
    Code:
    this.afterInvalidDrop(e, id);
    will not have the event parameter.

  4. #4
    Sencha User
    Join Date
    Oct 2011
    Location
    Sweden
    Posts
    36
    Vote Rating
    0
    JambaFun is on a distinguished road

      0  

    Default


    Hi enpasos,

    It's nice to see someone else studying the dd source code! This has been my main focus for the last week when building custom functionality for the TabPanel. I've had to resort to all kinds of mapping techniques (go pen and paper!) to wrap my head around how everything fits together. Most of the confusion comes from Ext.dd.DragSource overriding so much of its ancestors thus heavily altering code flow.

    When looking at onInvalidDrop I come to the same conclusion as you. When called from Ext.dd.DragDropManager.fireEvents() it gets the wrong parameters. This only happens when you drop your drag object outside of a drop zone within the same group (i.e. !dropEvts.length). If you drop it inside a drop zone the code that evaluates whether it's invalid or not is in Ext.dd.DragSource.onDragDrop(). This method in turn calls onInvalidDrop with the correct parameters.

    If you really depend on something to happen inside the afterInvalidDrop-callback I would override DragSource.onDragDrop() to work around the problem.

  5. #5
    Ext JS Premium Member
    Join Date
    Mar 2008
    Location
    Germany, Frankfurt
    Posts
    142
    Vote Rating
    1
    enpasos is on a distinguished road

      0  

    Default


    Thanks JambaFun for the hint on the work around. Good luck with your TabPanel work.

    I think the bug is obvious ... I'm sure the Sencha team will find a good solution ... I hope for one in ExtJS4.1.

  6. #6
    Sencha Premium Member skirtle's Avatar
    Join Date
    Oct 2010
    Location
    UK
    Posts
    3,590
    Vote Rating
    322
    skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future skirtle has a brilliant future

      0  

    Default


    A quick check suggests that this hasn't changed since ExtJS 3, possibly even earlier. While I agree it appears by inspection to be incorrect you have to wonder why such an 'obvious' bug hasn't caused serious issues for many more users. It makes me wonder whether there's a less than obvious reason why this bug doesn't cause problems in practice. It really needs a clear test case that shows that this problem exists in practice, not just in theory.

    The DD code in ExtJS dates right back to the library's origins as a YUI spin off. Indeed, all the DD source files still include a copy of the original Yahoo! BSD license. I think everybody agrees it's horrid but on a pragmatic level it does seem to work. Long term it's due for a proper rewrite to bring it up to the standard of the rest of the library but for the time being I think it's very much a case of only fixing the most serious issues rather than risking making it worse trying to polish up the edge cases.

  7. #7
    Ext JS Premium Member
    Join Date
    Mar 2008
    Location
    Germany, Frankfurt
    Posts
    142
    Vote Rating
    1
    enpasos is on a distinguished road

      0  

    Default


    The test case:
    modify extjs-4.1.0-rc3\examples\dd\dragdropzones.js by adding on line after line 230:
    Code:
    ,afterInvalidDrop: function(e, id) { Ext.Msg.alert('afterInvalidDrop', 'e = '+ e);  }
    Then drag a patient address and release it on the Patient Panel -> Bug results in no event object.
    (If you drag to the Hopitals Panel the event object is not empty).

    My expectation:
    A documented functionality does not work as documented. The problem's location in the code is identified. I expect a fix in the code or in the documentation if the documented feature is not intented to be delivered for what ever reason. To intensionally "promise functionality and not to deliver it" I would not expect from the Sencha team.

  8. #8
    Ext JS Premium Member
    Join Date
    Mar 2008
    Location
    Germany, Frankfurt
    Posts
    142
    Vote Rating
    1
    enpasos is on a distinguished road

      0  

    Default


    posted it as a bug

Thread Participants: 2