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

selection detection / handling issues #25

Closed
mrckzgl opened this issue Dec 17, 2020 · 0 comments · Fixed by #26
Closed

selection detection / handling issues #25

mrckzgl opened this issue Dec 17, 2020 · 0 comments · Fixed by #26

Comments

@mrckzgl
Copy link
Contributor

mrckzgl commented Dec 17, 2020

Preface

Hi, first thanks for this great library and the extensive work which was put in. Also of course thanks for making it publicly available. I am working on a small annotation tool which incorporates your library. We only use the Editor class of your library. There are some caveats which we detected and for some I also found workarounds and fixes, which I'd like to contribute upstream. So, if things work well there might be issue reports and also pull requests following :-)

Issue

I've noticed that selected nodes detection does not work correctly in all cases in Firefox. This is due to https://bugzilla.mozilla.org/show_bug.cgi?id=957724#c10
In contrast to chromium, in ff, depending where a user clicks it might be the case that the selection range starts at the span previous to the first selected char with startOffset = 1 (instead of the start span with startOffset = 0). This can be reproduced by double clicking a word in the editor and then add an annotation. The annotation will include the spaces surrounding the word.

I will attach a pull request to fix this. The pull request also fixes that getSelectionNodes() will crash if selection is outside of editor div or if the selection is empty. In the project we develop we need to detect if selection is cleared. Currently the Editor does not notify the selectors in that case. It would be nice if this could be added. I am happy to include this into the pull request (don't know if this has any side effects to the other parts of the library which I don't use, hence I didn't include this for now)
To fix that, I simply call updateSelectors() in any case in handleMouseUpEvent() and let updateSelectors() also notify the selectors if selection is null.

best
Martin

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

Successfully merging a pull request may close this issue.

1 participant