Skip to content
This repository was archived by the owner on Jun 15, 2021. It is now read-only.
This repository was archived by the owner on Jun 15, 2021. It is now read-only.

Selection.range collapses when selecting the last word of a paragraph #526

Open

Description

@sauerbraten

In Firefox 62.0.3, I can't select the last word of a paragraph, then use 'createLink' to wrap that word in an anchor tag. (You can reproduce this in the demo.) It works when using just document.execCommand(), so I suspected it had to do with the createLink-patch in the core plugin.

Turns out it is a bug in Selection():

  1. Firefox's document.getSelection() returns a selection with anchorNode set to a #text node containing the word and focusNode set to the parent

    node (and focusOffset set so that the selection ends after the #text node child (= the anchorNode).

  2. Later in the code, the isBefore() helper function is used which does not take into account that child nodes of N come after N in the document, i.e. Selection incorrectly assumes the selection is backwards.
  3. Finally, creating a new Range and calling setEnd() with the incorrectly swapped start and end results in a collapsed range as per https://developer.mozilla.org/en-US/docs/Web/API/Range/setEnd.

I suggest fixing this by checking isBefore() and Node.contains() in Selection() to prevent wrongly inverting the range. I'll prepare a pull request containing this fix.

Activity

  1. jukecraft commented on Oct 30, 2018

    @jukecraft

    @sauerbraten thank you for bringing this up. Just a note before you go to work, you might want to know about our current text editor plans: #512 (comment)

  2. sauerbraten commented on Oct 30, 2018

    @sauerbraten
    Author

    Thanks for the quick feedback @franziskas! Interesting, you don't happen to have more information about the timeline for the integration of ProseMirror?

    I'll create the PR anyway, but have trouble writing a test for this or getting the demo to work correctly (npm run build and ./build-cjs.sh in a fresh checkout produced a buggy demo for me). I'm also not sure how to port the old selection.spec.js tests.

  3. danburzo commented on Oct 30, 2018

    @danburzo
    Contributor

    @sauerbraten Could you outline the concrete reproducing steps? I'm on Firefox 63 and at first brush I couldn't reproduce it, and I'm curious to see if it's a FF 62 thing.

  4. sauerbraten commented on Oct 30, 2018

    @sauerbraten
    Author

    @danburzo sure:

    1. remove all content from the demo window
    2. put in just 'foo'
    3. the output area should now show <p>foo<br></p> and nothing else
    4. double click the foo to select it
    5. if you want, run document.geSelection() in the console to verify that the endNode is a p node, for example, I get Selection { anchorNode: #text, anchorOffset: 0, focusNode: p, focusOffset: 1, isCollapsed: false, rangeCount: 1, type: "Range", caretBidiLevel: 0 }
    6. click the Link button, type "http://a.com" into the modal (or anything really)
    7. confirm with enter key or by clicking OK
    8. output area will now show <p><a href="http://a.com">http://a.com</a>foo<br></p> and the contenteditable area reflects this

    Looking into these buggy behaviors some more, we found something else (which is probably unrelated but also occurs with links, so I'll mention it here):

    The state of the Link button depends on how a link is selected. Double clicking the a.com link created above will cause in the Link button showing as "inactive" (?), i.e. pressing it will not pre-fill the modal with the link's URL. This is also the case when selecting the entire link by moving the cursor from outside the link (| a.com) to the start ( |a.com) (or end), then holding Shift and using the right (or left) arrow key to select all the characters of the link. Doing so results in the same "inactive" Link button, even though a link is now selected. Selecting the whole link using Shift and arrow keys works when the cursor's last position before selecting was inside the link, i.e. when you go from a|.com to |a.com and then select to the right.

    For a custom linking button we use

    const isLink = node => node && node.nodeName.toLowerCase() === 'a';
    const getLinkParent = sel => sel.getContaining(isLink);
    command.queryEnabled = function () {
        let sel = new scribe.api.Selection();
        return !getLinkParent(sel) && !sel.range.cloneContents().querySelector('a');
    };
    

    to disable the button when the selection is inside a link, or a link is (even partly) selected. Maybe this could be used as command.queryState for the createLink command?

  5. jukecraft commented on Jan 30, 2019

    @jukecraft

    Thanks for the quick feedback @franziskas! Interesting, you don't happen to have more information about the timeline for the integration of ProseMirror?

    @sauerbraten I apologise for the long delay in replying - we've now published a blog post that should explain in detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions