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

Caret Position Rendering in Howling Mad #1138

Open
petrosalema opened this issue Mar 21, 2014 · 37 comments
Open

Caret Position Rendering in Howling Mad #1138

petrosalema opened this issue Mar 21, 2014 · 37 comments
Labels
Milestone

Comments

@petrosalema
Copy link
Member

  • Incorrect rendering adjacent to insignificant whitespace (\n at end of paragraph <p>).
<p>
    Donec et mollis dolor.  Praesent et diam eget libero egesta
    mattis sit amet vitae augue.  Nam tincidunt congue enim, u
    porta lorem lacinia consectetur. Donec ut libero sed arc 
    vehicula ultricies a non tortor*[]
</p>

caret-positioning

Notice how the caret is rendered (tall black bar) when having clicked to the right of the asterix at "tortor*".

  • Incorrect rendering between void elements (including <br>, <hr>, <img>, etc.).
<p>
    one 
    <br>
    <br>
    {}  
    <br>
    <br>
    two 
</p>

caret-position-2

... same problem in empty paragraphs.

<p>{}<br></p>
  • Incorrect absolute position when scrolling inside iframes.
  • Cross-browser support.
@deliminator
Copy link
Contributor

Please note that the incorrect absolute positioning with scrolling inside iframes occured when rendering the wikidocs telepointer with tinymce (tinymce replaces a textbox with an iframe).

@evindor
Copy link

evindor commented Mar 27, 2014

Could you please repeat the above, but with current and expected result?

@deliminator
Copy link
Contributor

This is as much as I know: when the Aloha cursor/wikidocs telepointer was rendered inside a TinyMCE iframe that had scrollbars, the place the cursor was rendered at was off approximately by the same distance that you could scroll up and down.

You can start tackling this issue by trying to reproduce it: create a new demo wikidocs-api/simple-php/tinymce.php in the wikidocs-api repository and implement it like the ckeditor.php demo, except for TinyMCE. Try to get the scrollbars on the editable somehow (I don't know how), and see if you can make the wikidocs telepointer's absolute positioning to be off by the scrolling distance.

Once you can reproduce it with wikidocs, you can try to reproduce it without wikidocs, just using the Aloha selection.

@evindor
Copy link

evindor commented Mar 28, 2014

@deliminator, @petrosalema at first i wanted to tackle this

Incorrect rendering adjacent to insignificant whitespace

and this

Incorrect rendering between void elements

so what i'm asking about is what is expected result here? :

<p>
    Donec et mollis dolor.  Praesent et diam eget libero egesta
    mattis sit amet vitae augue.  Nam tincidunt congue enim, u
    porta lorem lacinia consectetur. Donec ut libero sed arc 
    vehicula ultricies a non tortor*[]
</p>

or here:

<p>{}<br></p>

@petrosalema
Copy link
Member Author

@evindor I've added screenshots to the issue.

@draftkraft
Copy link
Contributor

@evindor The expected result is most of the times what MS Word does. In the comment above resutl should be:

  1. Collapsed selection alway in the same line and right after the last character of "tortor*"
  2. p br p according spec is 1 empty line, thus cursor in that line (note in IE 2 lines cursor in first line)

@evindor
Copy link

evindor commented Mar 28, 2014

@draftkraft @petrosalema @deliminator thanks guys, i was able to reproduce the issue, digging deeper now.

@evindor
Copy link

evindor commented Mar 31, 2014

So what i got so far is that bound function in ranges module is the one that cause problems in cursor rendering. I'm experimenting, in this commit i've tried to fix cursor rendering at the end of <p> and succeed.

For visually empty nodes such as <p><br></p> getBoundingClientRect returns zeros so it should be hacked around.

@deliminator
Copy link
Contributor

TypeError: CSS2Properties doesn't have an indexed property setter. maps.js:127

To make this work in wikidocs I just refactored selections.js to set the properties explicitly (without an Maps.extend)
caret.style['top'] = box.top + topDelta + 'px';
caret.style['left'] = box.left + leftDelta + 'px';
caret.style['height'] = box.height + 'px';
caret.style['width'] = '2px';
caret.style['display'] = 'block';
Please fix it also this way in Aloha and get rid of the Maps.extend.

evindor added a commit to evindor/Aloha-Editor that referenced this issue Apr 1, 2014
@evindor
Copy link

evindor commented Apr 1, 2014

@deliminator the commit above fixes

TypeError: CSS2Properties doesn't have an indexed property setter. maps.js:127

evindor added a commit to evindor/Aloha-Editor that referenced this issue Apr 1, 2014
deliminator added a commit that referenced this issue Apr 1, 2014
evindor added a commit to evindor/Aloha-Editor that referenced this issue Apr 1, 2014
evindor added a commit to evindor/Aloha-Editor that referenced this issue Apr 2, 2014
evindor added a commit to evindor/Aloha-Editor that referenced this issue Apr 2, 2014
evindor added a commit to evindor/Aloha-Editor that referenced this issue Apr 2, 2014
evindor added a commit to evindor/Aloha-Editor that referenced this issue Apr 2, 2014
evindor added a commit to evindor/Aloha-Editor that referenced this issue Apr 2, 2014
evindor added a commit to evindor/Aloha-Editor that referenced this issue Apr 2, 2014
evindor added a commit to evindor/Aloha-Editor that referenced this issue Apr 2, 2014
evindor added a commit to evindor/Aloha-Editor that referenced this issue Apr 3, 2014
evindor added a commit to evindor/Aloha-Editor that referenced this issue Apr 3, 2014
evindor added a commit to evindor/Aloha-Editor that referenced this issue Apr 3, 2014
@petrosalema
Copy link
Member Author

We also still have issues with consecutive <br/> elements:

aloha_editor_-_2014-05-15_18_fotor

Attempting to click into the white space that is provided by the 3 consecutive <br/> elements results in the caret being placed in the paragraph above.

@petrosalema
Copy link
Member Author

The caret is rendered out of place in the following situations too:

<p style="padding:20px">{}<br/></p>

and

<p style="border:20px solid red">{}<br/></p>

The caret is in front of the line/out of the box.
This is because our aloha.ranges.box() function does not take padding and border width into account.

#boxmodel

@evindor
Copy link

evindor commented Jun 3, 2014

@petrosalema
Copy link
Member Author

Also discovered this:

<ul>
  <li>foo</li>
  <li><b>{}<i></i></b></li>
</ul>
bar

to also be a problem.

Caret is rendered awkwardly to the left of "bar" text instead of above it in the list element under "foo".

@petrosalema
Copy link
Member Author

node.getBoundingClientRect/CSS hack → #1199

@evindor
Copy link

evindor commented Jun 10, 2014

Fixes in #1199:

  • <p style="padding:20px">{}<br/></p>
  • <p style="border:20px solid red">{}<br/></p>
  • <p><br><br>{}<br></p> (consecutive <br> elements)

Also i suppose that replacing offset logic with getBoundingClientRect() might fixed something else.

@petrosalema
Copy link
Member Author

@evindor #1199 takes us a good way forward.
There are a couple of edge cases that still cause trouble, however.
I've added 2 failing test cases → 34ec202

@evindor
Copy link

evindor commented Jun 10, 2014

@petrosalema actually if you try to manually place the caret in those two cases - it works. Please look at this commit i just made - tests are passing. I think there is no way to place a cursor like this: ...<br>{}</p>.

@evindor
Copy link

evindor commented Jun 10, 2014

We still have this issue though.

@petrosalema
Copy link
Member Author

@evindor When I go into the sandbox, and place the cursor right at the end of the pink "src" in the H1 heading.

My selection becomes:

<h1>Aloha Editor: <span style="color:#df215a">src{}</span></h1>

When I then hold SHIFT and hit ENTER. A new line is created, and my selection becomes:

<h1>Aloha Editor: <span style="color:#df215a">src<br><br>{}</span></h1>

You will observe that the caret is at an awkward place.

@petrosalema
Copy link
Member Author

If you set the caret to the right end of an list item and do SHIFT+ENTER, same problem.
SHIFT+ENTER at the very end of the editable also manifests the same problem.

@petrosalema
Copy link
Member Author

@evindor I've discovered another edge-case:

<div class="aloha-editable">
    one
    <div draggable="true" class="aloha-block">Aloha Block</div>{}<div>
        <p>two</p>
        <p>three</p>
    </div>
</div>

screen shot 2014-06-20 at 11 06 34

This is the result when you click to the right of the grey block.

@petrosalema
Copy link
Member Author

There is also this (the caret is rendered on the wrong side of the block):

<ul>
  <li>one</li>
  <li><div draggable="true" class="aloha-block" contenteditable="false">Aloha Block</div>{}</li>
  <li>two</li>
</ul>

screen shot 2014-06-20 at 11 49 29

@petrosalema
Copy link
Member Author

Another edge case: When positioning the caret right in from of an text-level semantic element that spans multiple lines.

<div class="aloha-editable">on{}<b>e<br>t</b>wo<br></div>

screen shot 2014-07-24 at 17 15 45

@evindor
Copy link

evindor commented Jul 25, 2014

@petrosalema what do you think about this solution?

@evindor
Copy link

evindor commented Jul 25, 2014

Seems like it's passing tests, and everything works, but i'm worried that this may result in other edge cases.

@evindor
Copy link

evindor commented Jul 25, 2014

this should fix all 3 edge cases mentioned above.

@petrosalema
Copy link
Member Author

@evindor, your fix seems to solve the last problem I posted, but does not appear to address the earlier two: #1138 (comment) and #1138 (comment).

@evindor
Copy link

evindor commented Jul 25, 2014

@petrosalema hmm. Are you sure? That block has display: block (or table-cell which i believe is equal in this case) so it's kind of logical how the cursor stands right now.

@evindor
Copy link

evindor commented Jul 25, 2014

With inline-block positioning it works as expected.

petrosalema pushed a commit that referenced this issue Jul 28, 2014
@petrosalema
Copy link
Member Author

There are still some problems I encounter.

Given this:

<div class="aloha-editable">
<p>one</p>
<div draggable="true" class="aloha-block" contenteditable="false">Aloha Block</div>{}<ul><li>two</li><li>three</li></ul>
</div>

the caret is rendered thus:

screen shot 2014-07-29 at 13 32 10

This is infact the same issue as this previous one.

@petrosalema
Copy link
Member Author

We have this issue as well.

Given:

<div class="aloha-editable">
<p>one
<br>two
<br>{}
<br></p>
</div>

The caret is rendered according to the <p> element:

screen shot 2014-07-30 at 18 16 06

@evindor
Copy link

evindor commented Jul 31, 2014

Could you please tell me how to reproduce the last issue? I wasn't able to set the cursor where it stands in your html. Then i used the following html:

"<div class="aloha-editable">
<p>one
<br>two
<br></p>
</div>"

set cursor after "two", pressed shift+enter, got:

"<div class="aloha-editable bm" style="min-height: 1em; cursor: text;">
<p>one
<br>two
<br>{}<br></p>
</div>"

with cursor standing where it should stand.

@evindor
Copy link

evindor commented Jul 31, 2014

I'm also confused with <div draggable="true" class="aloha-block" contenteditable="false">Aloha Block</div>{}<ul><li>two</li><li>three</li></ul>. Technically the caret is rendered correctly.
I guess the right way to render caret in this case is right after the block, to do this:

// ranges.js:729
-       var node = Boundaries.nodeAfter(boundary)
-               || Boundaries.nodeBefore(boundary);
+       var node = /*Boundaries.nodeAfter(boundary)
+               || */Boundaries.nodeBefore(boundary);

        if (node && !Dom.isTextNode(node)) {
            rect = boundingRect(node);
            if (rect) {
                return {
                    top    : rect.top + topOffset,
-                   left   : rect.left + leftOffset,
+                   left   : rect.left + rect.width + leftOffset,
                    width  : rect.width,
                    height : rect.height
                };
            }
        }

This is an edge case so i think we should find the property we could check and decide when to nodeBefore first.

@Jotschi Jotschi added this to the 2.0.x milestone Oct 30, 2014
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

5 participants