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

select deletion of own content #42

Closed
johanneswilm opened this issue Jan 29, 2013 · 4 comments
Closed

select deletion of own content #42

johanneswilm opened this issue Jan 29, 2013 · 4 comments

Comments

@johanneswilm
Copy link
Contributor

If a user selects and delete his own contents, this sometimes works and sometimes doesn't.

If it is part of a larger selection which also includes othr nodes, it works. If the user only selects all or part of a text he has written himself, it doesn't work.

I have a fix for this here:

johanneswilm@4abbcb6

In this line:

if (this._getNoTrackElement(elem) || this._currentUserIceNode(elem) || elem.nodeType === ice.dom.TEXT_NODE && this._currentUserIceNode(elem.parentNode))

It basically says as a third option "if the element is a text node, then check the parent element if it is an ice element of the current user.

@delambo
Copy link
Member

delambo commented Jan 30, 2013

I ticketed this issue internally and am looking into this. Your solution doesn't cover some cases. I should have a more robust solution on master soon and will release the fix with 0.4.2.

Feel free to submit some of your patches as pulls if you like.

@johanneswilm
Copy link
Contributor Author

Ah yes, I noticed that as well. Basically it doesn't work if it is inside yet another node. Something like a getParentInsertNode would be needed to find the first insert node going upward in the tree. I will pull that as well as the webkit compatible _handleEnter solution when you commit them. Then we should probably look at whether you guys are interesting in support anything beyond normal paragraphs at all. if not, then we won't be able to merge the two branches completely, but we should try to get them to be as close as possible. If you are OK with supporting other elements, then we should diff between your and my branch and see how much more difference there is and I will try to break it all down into individual patches.

@johanneswilm
Copy link
Contributor Author

I tried to fix this here: johanneswilm@824831d

However, this is mostly geared towards stuff that inserts nodes. Given that ICE only has insert and delete as change types, it works under these conditions.

@delambo Is this the direction you were thinking in?

@johanneswilm
Copy link
Contributor Author

This has been fixed with the pull #49 . Nice one!

delambo pushed a commit that referenced this issue Feb 14, 2013
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

2 participants