Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(At least) Drag and Drop "Drop Based Coding" broken #1492

Closed
jmbreuer opened this issue Dec 13, 2013 · 17 comments
Closed

(At least) Drag and Drop "Drop Based Coding" broken #1492

jmbreuer opened this issue Dec 13, 2013 · 17 comments

Comments

@jmbreuer
Copy link

While trying to find out why my application would never run any drag:drophit event handlers, I found that the current (3.14.0) "Using Drop Based Coding" example would also not run correctly on any browsers here - the drop:hit handler is never executed.

This is on Linux (gentoo) x86_64, with all of:

  1. Chromium 31.0.1650.63 (238485)
  2. Firefox ESR 24.1.1
  3. Konqueror 4.10.5

Using a traditional mouse as the input device (no touch capability).

The same happens with Safari 7.0 (9537.71) on MacOS X 10.9.

All show exactly the same behavior, reaction to dropping in the target area is exactly the same as dropping outside the target area - i.e. the drag node snaps back to its original position, the drop target loses its hover color (orange back to blue) and nothing else happens (as I understand it, the drop target should show data about the dragged element).

In contrast, the "Drag - Interaction Groups" example works correctly here; although in my own code loosely based on that example the handler for drag:drophit does not execute either.

@jmbreuer
Copy link
Author

It's still broken in exactly the same way with 3.14.1.

@hacklschorsch
Copy link
Contributor

Also happens on Opera 12.16 (Opera/9.80 (X11; Linux x86_64) Presto/2.12.388 Version/12.16) with above link from the docs.

@hacklschorsch
Copy link
Contributor

I downloaded v3.8.0 (a year old) and tried w/ chromium (Version 30.0.1599.114 Ubuntu 13.10 (30.0.1599.114-0ubuntu0.13.10.2) locally, same broken behavior.

@hacklschorsch
Copy link
Contributor

The bug seems to not yet exist in v3.3.0 (three years old). There above example works.

I downloaded the oldish YUI and renamed drop-code_clean.php to drop-code_clean.php.html. When opening that in my browser the example works (it displays the text "Dropped: id: drag5, color: purple, size: x-large, price: $15.00" in the drop target). The only PHP seemed to be some cache buster code. Beware, the directory structure of 3.3.0 is somewhat different from the more recent distributions.


Just fav'd a tweet today saying

reminder: if you're not writing unit tests, you're writing legacy code /via @mfeathers

@hacklschorsch
Copy link
Contributor

I would love to do a git bisection on this, but I haven't ever built a yui locally.

For the unit test... I can only think of a UI test, but the last time I checked selenium webdriver (a year ago?) their dragging and dropping also was broken. Sigh.

@hacklschorsch
Copy link
Contributor

Oh man, just saw your Selleck testing methodology. YUI is so full of awesomeness. @davglass, you're missed.

@hacklschorsch
Copy link
Contributor

@jmbreuer quoted my comparing git to heroine once before IIRC ;)

did a manual bisection from v3.3.0 to HEAD.

1a55a78 is the first bad commit

it says. Now that's around v3.7.3, and 1a55a78 is a merge commit and I don't know what to do next.

@hacklschorsch
Copy link
Contributor

well d'uh of course the largish merge wasn't the problem. It's d93edc5 ("Fixes scrollig issues in Safari 6 for DD (a workaround, node issue to be filed after")

@hacklschorsch
Copy link
Contributor

Well or isn't it. I reverted d93edc5 and rebuilt dd using shifter, but the problem persists. Too tired today, will try my luck some other time.

@hacklschorsch
Copy link
Contributor

OK, both parents (7e28b55 and fb0937e) of the broken largish merge commit (1a55a78) seem to be OK here. So the breakage really seems to be in the merge.

@hacklschorsch
Copy link
Contributor

Both parents work, but the child doesn't work with either parent version of drag.js. Seems like we have to look more closely at the other files.

@hacklschorsch
Copy link
Contributor

1a55a78 with src/dd/js/ddm-drop.js from fb0937e works.

@hacklschorsch
Copy link
Contributor

1a55a78 with src/dd/js/ddm-drop.js with all calls to Y.Array.each replaced by Y.each works. The latter "Supports arrays, objects, and NodeLists"

@hacklschorsch
Copy link
Contributor

ddm-drop.js line 122

//TODO Change array/object literals to be in sync..
validDrops: [],

d'uh

@hacklschorsch
Copy link
Contributor

otherDrops is an Object Literal, not an Array.

(I still like dynamic languages.)

@hacklschorsch
Copy link
Contributor

Maybe Y.Array.each should throw an Exception if it's not fed an array? Or is the check too expensive?

hacklschorsch added a commit to hacklschorsch/yui3 that referenced this issue Jan 23, 2014
At ~ 3.7.3 a regression was introduced by 1a55a78 that broke
"Drop Based Coding" by using Array.each() for otherDrops which
is an Object Literal.
hacklschorsch added a commit to hacklschorsch/yui3 that referenced this issue Jan 23, 2014
hacklschorsch added a commit to hacklschorsch/yui3 that referenced this issue Jan 25, 2014
hacklschorsch added a commit to hacklschorsch/yui3 that referenced this issue Jan 25, 2014
@ghost ghost assigned okuryu Jan 28, 2014
@okuryu
Copy link
Member

okuryu commented Jan 29, 2014

#1573 has been merged.

@okuryu okuryu closed this as completed Jan 29, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants