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

ng-sortable doesn't work in Safari 8 #285

Closed
demisx opened this issue Feb 18, 2015 · 18 comments
Closed

ng-sortable doesn't work in Safari 8 #285

demisx opened this issue Feb 18, 2015 · 18 comments
Labels
Milestone

Comments

@demisx
Copy link

demisx commented Feb 18, 2015

I am on Mac Yosemite 10.10.2. The ng-sortable doesn't work in Safari 8. You can see this with your demo fiddle http://jsbin.com/naduvo/1/edit?html,js,output

@evillemez
Copy link
Contributor

I can confirm this - same versions of OS X/Safari. It's working in Chrome 40 for me.

Unfortunately it's not clear at all what's wrong. No error messages, it just doesn't do anything.

@evillemez
Copy link
Contributor

Actually, this probably isn't related to ng-sortable specifically, but the underlying sortable implementation. None of the demos at http://rubaxa.github.io/Sortable/ are sorting in Safari 8 either.

@RubaXa RubaXa added the angular label Feb 19, 2015
@evillemez
Copy link
Contributor

Lots of console.logs later, I've traced it (I think) to this line:
https://github.com/RubaXa/Sortable/blob/master/Sortable.js#L662

In Chrome, this condition is met, but not in Safari 8. If I can figure out what's going on I'll submit a PR.

@evillemez
Copy link
Contributor

@RubaXa I saw you added an angular label - it's not really angular that's the problem though, it's Sortable.js in general that is broken in Safari 8. We just discovered it while trying to use ng-sortable. Should I start a new issue for this since the name is misleading?

@RubaXa
Copy link
Collaborator

RubaXa commented Feb 19, 2015

Yes, you're right, it just does not work 0_o.

@demisx
Copy link
Author

demisx commented Feb 19, 2015

@evillemez 👍

@RubaXa
Copy link
Collaborator

RubaXa commented Feb 19, 2015

https://github.com/RubaXa/Sortable/blob/master/Sortable.js#L485 — (!) effectAllowed always all 💩

@evillemez
Copy link
Contributor

I'm not sure if that's it... I found some odd behavior in Safari related to the line I mentioned earlier:

https://github.com/RubaXa/Sortable/blob/master/Sortable.js#L662

Logging both of the elements in that condition gives me different results in Safari vs Chrome.

Safari 8:

  • dragEl.nextSibling: <!-- end ngRepeat: skill in skills -->
  • nextEl: <!-- end ngRepeat: skill in skills -->

But, in Chrome 40, logging both elements, I get:

  • dragEl.nextSibling: <li ng-repeat="skill in skills" class="ng-binding ng-scope">Foo</li>
  • nextEl: <!-- end ngRepeat: skill in skills -->

@evillemez
Copy link
Contributor

ack, sorry, editing message - put wrong info in it

@evillemez
Copy link
Contributor

So, in Safari 8, for both elements it has the comment nodes injected by ng-repeat

@RubaXa
Copy link
Collaborator

RubaXa commented Feb 19, 2015

Try dev-branch.

RubaXa added a commit that referenced this issue Feb 19, 2015
@RubaXa RubaXa added this to the v1.1.1 milestone Feb 19, 2015
@evillemez
Copy link
Contributor

@RubaXa Ah, you are my hero - that fixed it.

@RubaXa
Copy link
Collaborator

RubaXa commented Feb 19, 2015

Actually sorry that it happened, it's my fault that has not checked in Safari (although this is a bug in it).

@evillemez
Copy link
Contributor

No worries, still a good library, thanks for writing it :) Спасибо огромное!

@RubaXa RubaXa added the bug label Feb 19, 2015
@demisx
Copy link
Author

demisx commented Feb 19, 2015

Any way this could be covered in tests?

@RubaXa
Copy link
Collaborator

RubaXa commented Feb 19, 2015

Automatically? it's impossible, only manual testing.

@evillemez
Copy link
Contributor

Well, it's probably doable with Saucelabs, but it's not trivial to set up.

https://saucelabs.com

@RubaXa
Copy link
Collaborator

RubaXa commented Feb 19, 2015

Not sure that Selenium correctly reproduces all events with drag and drop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants