Opened 8 years ago
Closed 8 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 )
Steps to reproduce
- Open sample which contains Find and Divarea plugins (divrea sample in standard package).
- Type
New Paragraph added
- Open Find dialog and search for
New
- 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 8 years ago by
Status: | new → confirmed |
---|
comment:2 Changed 8 years ago by
Keywords: | Support added |
---|
comment:3 Changed 8 years ago by
comment:4 Changed 8 years ago by
Description: | modified (diff) |
---|
comment:5 Changed 8 years ago by
Description: | modified (diff) |
---|
comment:6 Changed 8 years ago by
Milestone: | → CKEditor 4.6.1 |
---|
comment:7 Changed 8 years ago by
Owner: | set to Tade0 |
---|---|
Status: | confirmed → assigned |
comment:8 Changed 8 years ago by
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 8 years ago by
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 8 years ago by
Status: | assigned → review |
---|
Fix confirmed to work on IE8, IE11, Chrome, Firefox, Edge.
Added manual test.
Changes pushed to branch:t/14869.
comment:11 Changed 8 years ago by
Milestone: | CKEditor 4.6.1 → CKEditor 4.6.2 |
---|
Moving to 4.6.2 minor release, as 4.6.1 is mostly about polishing 4.6.0.
comment:12 Changed 8 years ago by
Status: | review → review_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.
comment:13 Changed 8 years ago by
Status: | review_failed → review |
---|
Moved the test to /tests/plugins/find/manual/textnodesplit
and modified the description so that the error is reproducible.
comment:14 Changed 8 years ago by
Status: | review → review_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 8 years ago by
Status: | review_failed → review |
---|
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 8 years ago by
Resolution: | → fixed |
---|---|
Status: | review → closed |
Ok, so let's skip the unit test then, everything else LGTM. Fixed with git:35cb196.
Based on information from user who originally reported this ticket, I have debugged this issue. It seems the problem is
editor.focus();
inonHide
method in find.js file.When debuging code manually error is not thrown so a good workaround for this issue is warping
editor.focus()
inStill after calling
editor.getSelection().selectRanges( [ range ] );
start/end offsets show 0/1 (not 03). In order to fix offsets you need to calleditor.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.