Opened 2 years ago

Closed 2 years ago

#14869 closed Bug (fixed)

JS error thrown with Find and Divarea plugins used together.

Reported by: Jakub Ś Owned by: Tade0
Priority: Normal Milestone: CKEditor 4.6.2
Component: General Version: 4.0
Keywords: Support Cc:

Description (last modified by Jakub Ś)

Steps to reproduce

  1. Open sample which contains Find and Divarea plugins (divrea sample in standard package).
  2. Type New Paragraph added
  3. Open Find dialog and search for New
  4. Close the dialog with close button.

Expected result

No error is thrown.

Actual result

JS error is thrown:
Message:
Firefox: IndexSizeError: Index or size is negative or greater than the allowed amount
Chrome: IndexSizeError: Failed to execute 'setStart' on 'Range': The offset 19 is larger than or equal to the node's length (3).
Line:1933
Path: ckeditor-dev/core/selection.js

Other details (browser, OS, CKEditor version, installed plugins)

Problem can be reproduced in all browsers.

Change History (16)

comment:1 Changed 2 years ago by Jakub Ś

Status: newconfirmed

comment:2 Changed 2 years ago by Jakub Ś

Keywords: Support added

comment:3 Changed 2 years ago by Jakub Ś

Based on information from user who originally reported this ticket, I have debugged this issue. It seems the problem is editor.focus(); in onHide method in find.js file.

When debuging code manually error is not thrown so a good workaround for this issue is warping editor.focus() in

CKEDITOR.tools.setTimeout( function() {
  editor.focus();
}, 0, this );

Still after calling editor.getSelection().selectRanges( [ range ] ); start/end offsets show 0/1 (not 03). In order to fix offsets you need to call editor.getSelection().getRanges()[0].moveToRange(range); but you should rather not be doing that in a first place so this is another workaround just for offsets.

comment:4 Changed 2 years ago by Jakub Ś

Description: modified (diff)

comment:5 Changed 2 years ago by Jakub Ś

Description: modified (diff)

comment:6 Changed 2 years ago by Jakub Ś

Milestone: CKEditor 4.6.1

comment:7 Changed 2 years ago by Tade0

Owner: set to Tade0
Status: confirmedassigned

comment:8 Changed 2 years ago by Tade0

What is happening here is that when the highlight is removed the adjacent text nodes remain split even though they are adjacent.

Making sure that adjacent text nodes are merged after removing the highlight should fix this problem.

comment:9 Changed 2 years ago by Tade0

Putting editor.focus() at the end seems to do the trick. I'll test this on other browsers than Chrome to see if it really works.

comment:10 Changed 2 years ago by Tade0

Status: assignedreview

Fix confirmed to work on IE8, IE11, Chrome, Firefox, Edge.

Added manual test.

Changes pushed to branch:t/14869.

comment:11 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.6.1CKEditor 4.6.2

Moving to 4.6.2 minor release, as 4.6.1 is mostly about polishing 4.6.0.

comment:12 Changed 2 years ago by Tomasz Jakut

Status: reviewreview_failed

Current test is not failable. To correctly reproduce the issue, user must create new paragraph with at least two words in the editor that has already some content.

Tests could be also moved to other tests for "Find" plugin.

Last edited 2 years ago by Tomasz Jakut (previous) (diff)

comment:13 Changed 2 years ago by Tade0

Status: review_failedreview

Moved the test to /tests/plugins/find/manual/textnodesplit and modified the description so that the error is reproducible.

comment:14 Changed 2 years ago by Tomasz Jakut

Status: reviewreview_failed

Fix seems to be working fine, the manual test is also good now. However we're still missing some unit test. It would be nice to have one to avoid regressions in the future. This should be the last part of this ticket.

comment:15 Changed 2 years ago by Tade0

Status: review_failedreview

A unit test would be great, but designing one that would actually test something proved to be hard or perhaps impossible in this case.

comment:16 Changed 2 years ago by Tomasz Jakut

Resolution: fixed
Status: reviewclosed

Ok, so let's skip the unit test then, everything else LGTM. Fixed with git:35cb196.

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy