ckeditor / ckeditor5

Powerful rich text editor framework with a modular architecture, modern integrations, and features like collaborative editing.
https://ckeditor.com/ckeditor-5
Other
9.44k stars 3.69k forks source link

verticalNavigationHandler/isSingleLineRange works incorrectly for web certain web fonts #12196

Open Inviz opened 2 years ago

Inviz commented 2 years ago

There are fonts for which getDomRangeRects returns vertically overlapping ranges for lines going immediately one after another. IsSingleLineRange does not expect that, which causes arrow up in first multiline paragraph jump immediately to the beginning skipping lines.

I tested it inside a container that has isLimit. I think same thing happens with image before paragraph.

<container>
<paragraph>
[]Long paragraph line 1 <-- after arrow up cursor is here
Long para[]graph line 2 <-- should have been here here
Long para|graph line 3   <-- arrow up was pressed here
</paragraph>
</container>
<figure class='image'></figure>
<paragraph>
[]Long paragraph line 1 <-- after arrow up cursor is here
Long para[]graph line 2 <-- should have been here here
Long para|graph line 3   <-- arrow up was pressed here
</paragraph>

πŸ“ Provide detailed reproduction steps (if any)

  1. Use font like Saira from google fonts, use font-size 14px, line-height 24px
  2. Create multiline paragraph
  3. Place cursor somewhere in 3+ line
  4. Press arrow up

βœ”οΈ Expected result

  1. Cursor moves one line higher

❌ Actual result

  1. Cursor jumps to the beginning of a paragraph

❓ Possible solution

The solution is to improve isSingleLineRange to have more lax check expecting that range boxes may overlap (either allow some slack like 25%, or check if bottom overflowed top)

πŸ“ƒ Other details


If you'd like to see this fixed sooner, add a πŸ‘ reaction to this post.

Reinmar commented 2 years ago

Thanks for the detailed analysis :heart: If you came that far, did you perhaps also implement your solution?

Do you know why this specific font creates this problem? Is there something unusual about it? We wonder what's the frequency of this issue (I don't recall it reported so far).

Inviz commented 2 years ago

@Reinmar Hello! Well, I don't have a good solution. I may think about it, but I think you guys have a good spec suite and know-how to do it in a more bullet proof way.

I think in general even web fonts are not the issue here, it is possible to re-create the issue by having inline element (like strong inside paragraph) with line-height being higher than paragraphs. This will make the range box overlap. I think certain web fonts have pt size or whatever the internal font metric is higher than 100% of the box, cousing the issue appear even on regular texts.

However! I found a way to reproduce it easily right on official demo out of the box, no style changes required: https://ckeditor.com/ckeditor-5/demo/ The font you use also has this issue :)

Put the centered picture before a paragraph, and add more text so there's more than 2 lines. Put cursor on 3+ line and press up. The cursor will jump to the beginning of a paragraph. On the picture I selected the text, look closely - the highlighted line ranges overlap each other causing the issue.

Screen Shot 2022-08-04 at 13 18 14

The reason it was not reported is either lack of attention to details, or low expectations from the editor. I however love CKEditor and I'm scrutinizing it all the time. In my project I use CKEditor as more of a framework, rather than editor, so I do a lot of strange things, and I when I see weird behavior I usually think it's me :) But in this case I had to make sure.

Inviz commented 2 years ago

@Reinmar Can you confirm that you're seeing this on the official demo? Thanks :)

CKEditorBot commented 1 year ago

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

Inviz commented 1 year ago

Still interested

On Sat, Sep 30, 2023 at 1:18β€―PM CKEditor Bot @.***> wrote:

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

β€” Reply to this email directly, view it on GitHub https://github.com/ckeditor/ckeditor5/issues/12196#issuecomment-1741681978, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAVFB33DXGH5ITP2A7K4DX46TSTANCNFSM55IRMXDA . You are receiving this because you authored the thread.Message ID: @.***>

CKEditorBot commented 3 days ago

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.