The cursor always moves to the left of the image while clicking the image.

[zenglincient]

the cursor always moves to the left of the image while clicking the image.

version

1. etherpad V.1.8.0
2. ep_image_upload@1.0.13

system

• mac ox/window
• chrome v79.03

description

1. upload an image
2. click/move cursor to the right of the image
3. the cursor moves to the left of the image automatically

just like this

find the problem

In order to find what makes the cursor move, I find the code that makes the cursor change in etherpad/src/static/js/ace2_inner.js: 4013

the start.offset/ end. offset always be zero while clicking the image

                if (selection) {
                    isCollapsed = (selection.startPoint.node === selection.endPoint.node && selection.startPoint.index === selection.endPoint.index);
                    var start = pointToRangeBound(selection.startPoint);
                    var end = pointToRangeBound(selection.endPoint);

                    if ((!isCollapsed) && selection.focusAtStart && browserSelection.collapse && browserSelection.extend) {
                        // can handle "backwards"-oriented selection, shift-arrow-keys move start
                        // of selection
                        browserSelection.collapse(end.container, end.offset);
                        // console.trace();
                        // console.log(htmlPrettyEscape(rep.alltext));
                        // console.log("%o %o", rep.selStart, rep.selEnd);
                        // console.log("%o %d", start.container, start.offset);
                        browserSelection.extend(start.container, start.offset);
                    } else {
                        var range = doc.createRange();
                        range.setStart(start.container, start.offset);
                        range.setEnd(end.container, end.offset);

                        browserSelection.removeAllRanges();
                        browserSelection.addRange(range);
                    }
                }

Expect for your reply

[tiblu]

Sorry for the late reply, have missed all the ep_image_upload notifications.

Thanks for the very detailed report! We'll have a look at this and prioritize. We have 1.5 developers in the organization so it may not get a very high priority. We welcome all pull reqests!

[loorm]

Triage 23. This needs further investigation. Need to check if ep_copy_paste_images has the same issue.

[loorm]

Triage 24. The issue is known. This is related to how Etherpad calculates the position of the cursor and how images are added differently from text (as 0 chars). May not have an immediate way to fix this.

[KatiVellak]

Legally reviewed, no additions.

[JohnMcLear]

I imagine this is due to the way that blockElements are not being properly implemented in the plugin, I might be able to patch it relatively easily.

[JohnMcLear]

Narrowed it down to here :)

while (p.node.childNodes.length > 0) {
            // && (p.node == root || p.node.parentNode == root)) {
            if (p.index === 0) {
// This is where it's setting it to the firstChild..
              p.node = p.node.firstChild;
              p.maxIndex = nodeMaxIndex(p.node);
            } else if (p.index == p.maxIndex) {
              p.node = p.node.lastChild;
              p.maxIndex = nodeMaxIndex(p.node);
              p.index = p.maxIndex;
            } else { break; }
          }

I think it's due to the nodeType of an image, which is why text objects don't have this issue.

https://stackoverflow.com/a/11037234/695411

[JohnMcLear]

I spent a bit more time on this and didn't get a fix in, every plugin that brings in content this way suffers from this problem.

Essentially if you add content that isn't text (nodeType == 3) Etherpad assumes it's an empty line so skips over it in the editor.

ace2_inner and domline are the mostly likely candidates for landing a fix but the code here is pretty in depth so picking out a specific fix that doesn't have any negative impact is going to be tricky.

I'm trying to think of plugin specific changes we can make IE put a textnode with a background image but this feels like a hack. Another approach I considered is a custom html element with hidden text but then copy/paste into other editors wouldn't work well. Ultimately I think we need a few days just to isolate the offending line and nurture it appropriately to not shift the selection when node.localName === IMG. I'm not sure on the scope of the fix tho, if we might consider supporting other elements here and the repercussions of that, for example INPUT/BR elements would cause a hella weird UX so I feel like it's only image that it makes sense to have the caret be before and after.

The good thing about if we support IMG only here is that selection length is always 0 or 1 but never greater. IE 0 is before image and 1 is after image.

[ilmartyrk]

After playing around with it in ace_inner.js I could only manage to either have carret before image or after image, but not both options. Tried backtracking to find where the positioning is done, inside pointToRangeBound was where I started and added exception to return 1 instead of 0 for node.nodeName === 'IMG'. After I saw it working thought to add support both 0 and 1. But that is where it all fell apart. This Whole stack is trigered with interval and this means, that even if you force one number it will after that fall back to other if you try to support that too.

Found that the first updating of position is done in updateBrowserSelectionFromRep using selection.startPoint = getPointForLineAndChar(ss); So tried instead moving this IMG nodeName check inside getPointForLineAndChar to see if it is possible to add an exception that could finally support both 0 or 1. But after playing around with index and maxIndex values and adding conditions here and there I saw that the initial point of caret is always 1 as after image and then it got forced to 0 so it was impossible for me to come up with solution that could detect if user had tried moving the caret before image.

[loorm]

Triage 42. Closing issue, because unfixable by us. Will wait for EP to fix this, hopefully in 2.0.