Opened 3 years ago

Last modified 2 years ago

#14744 assigned Bug

[Safari] Failing embedbase tests

Reported by: Marek Lewandowski Owned by: kkrzton
Priority: Nice to have (we want to work on it) Milestone:
Component: General Version: 4.5.11
Keywords: Cc:

Description (last modified by Marek Lewandowski)

2 unit tests are failing in this test suite:

tests/plugins/embedbase/undo test undo and redo after creation
Error
Error: IndexSizeError: DOM Exception 1
tests/plugins/embedbase/undo test undo and redo after edition
Unexpected error: IndexSizeError: DOM Exception 1
Expected: undefined (undefined)
Actual:   undefined (undefined)
Error
Error: IndexSizeError: DOM Exception 1
tests/plugins/embedbase/undo test undo and redo after creation and edition
Unexpected error: IndexSizeError: DOM Exception 1
Expected: undefined (undefined)
Actual:   undefined (undefined)

It started to appear after #14539. However I've followed the steps that TC is doing manually and the issue does not appear to be there, so it's likely due to a way that the test is written.

Attachments (1)

14744.gif (2.6 MB) - added by kkrzton 2 years ago.

Change History (14)

comment:1 Changed 3 years ago by Marek Lewandowski

Description: modified (diff)
Status: newconfirmed

comment:2 Changed 2 years ago by Olek Nowodziński

There's Safari Technology Preview available in the App Store, which will soon become a default web browser in Mac OS. Please, consider it when testing.

comment:3 Changed 2 years ago by Tomasz Jakut

Owner: set to Tomasz Jakut
Status: confirmedassigned

comment:4 Changed 2 years ago by Tomasz Jakut

It seems that the issue is caused by the selecting bookmarks performed inside CKEDITOR.undoManager.prototype.redo. However I'm still investigating why Safari claims that the selection's range is too great when the content of the editor is restored correctly.

comment:5 Changed 2 years ago by kkrzton

Owner: changed from Tomasz Jakut to kkrzton

comment:6 in reply to:  5 Changed 2 years ago by kkrzton

The cause of this issue is the way the native selection works in Safari in some cases. Setting selection like e.g. [<p>Text</p>] in Safari results in selection like <p>[Text]</p> (start and end container is a text node instead of element node). The undo/redo plugin uses bookmarks (which use element address) to restore selection and it seems based on the bookmarks the selection is restored on a wrong element.

As for: However I've followed the steps that TC is doing manually and the issue does not appear to be there, so it's likely due to a way that the test is written.

I also tried to reproduce this issue manually using the failing unit test and manually following the steps in the code. When following exactly those steps, I am able to reproduce the issue/error so it looks like it is not the matter of the unit test

comment:7 Changed 2 years ago by kkrzton

There are few things which are the cause of this issue:

  • There is a document.getByAddress method which when given incorrect address (e.g. [ 0, 2 ] for <p><span>1</span><span>2</span></p>) returns the last found element in the path/address (so for the previous example it would be p, because there is no 3rd child of p).
  • The snapshot which is used for redo'ing has bookmark with address pointing to hidden element with aria-label (added in #14539). While selection is restored by undo plugin there is no longer this hidden element.

So when selection is restored by undo plugin it uses parent element of unexisting hidden element returned by getByAddress method (to which this outdated address was given). In Chrome this element is inline editor container so it can be focused. In Safari because of earlier mentioned difference in native selection (<p>[Text]</p> instead of [<p>Text</p>]) it is one of the elements inside editor which cannot be focused due to invalid offset (if this element could be focused, the test would not be failing, which of course does not mean it is a valid behaviour).

comment:8 Changed 2 years ago by kkrzton

As mentioned in a previous comments the snapshot used on redoing contains hidden element (used for aria-label and hiding real selection) which is removed while restoring selection. It results in setting a selection based on an outdated bookmark.

This behaviour was not introduced by #14539 and occurred earlier. Since #14539 the hidden container holds some text and the test started to behave differently in Safari (because of different native selection) so the issue started to be visible.

Changed 2 years ago by kkrzton

Attachment: 14744.gif added

comment:9 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.11CKEditor 4.6.0

We'll move this issue as we don't have the time to resolve it now and it has not been reported yet so doesn't look as severe issue.

comment:10 Changed 2 years ago by Tomasz Jakut

Milestone: CKEditor 4.6.0CKEditor 4.6.1

comment:11 Changed 2 years ago by Tomasz Jakut

Ignored in git:c0e4dc3.

comment:12 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.6.1
Priority: NormalNice to have (we want to work on it)

comment:13 Changed 2 years ago by Marek Lewandowski

Ignored test undo and redo after edition in git:bdf88a4d78e1432b7531505d00b380176e5cb060.

Last edited 2 years ago by Marek Lewandowski (previous) (diff)
Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy