Opened 5 years ago

Closed 5 years ago

#12257 closed Bug (fixed)

[Chrome] /tests/plugins/enter/list - one of tests throws error "Discontiguous selection is not supported. "

Reported by: Piotrek Koszuliński Owned by: Artur Delura
Priority: Normal Milestone: CKEditor 4.4.6
Component: General Version:
Keywords: Cc:

Description

Run: http://tests.ckeditor.dev:1030/tests/plugins/enter/list One of tests throw:

Discontiguous selection is not supported. selection.js:1974
CKEDITOR.dom.selection.selectRanges selection.js:1974
CKEDITOR.dom.range.select selection.js:1014
CKEDITOR.plugins.enterkey.enterBlock plugin.js:392
enter plugin.js:525
editor.addCommand.exec plugin.js:13
exec command.js:52
CKEDITOR.tools.extend.execCommand editor.js:825
assertEnter list:92

Tests pass, but we're apparently trying to do something incorrect.

Change History (12)

comment:1 Changed 5 years ago by Piotr Jasiun

Status: newconfirmed

comment:2 Changed 5 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.4.4CKEditor 4.4.5

comment:4 Changed 5 years ago by Piotrek Koszuliński

Thanks Artur for finding this.

comment:5 Changed 5 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.4.5CKEditor 4.4.6

comment:6 Changed 5 years ago by Artur Delura

Owner: set to Artur Delura
Status: confirmedassigned

comment:7 Changed 5 years ago by Artur Delura

Changes in branch:t/12257. Simply when there is added range into relection while already exists one Chrome shows warning. To prevent this situation we have to clean reanges before adding one. In git:ce84cc4 I added wrapper function (this commit is optional).

comment:8 Changed 5 years ago by Artur Delura

Status: assignedreview

comment:9 Changed 5 years ago by Piotrek Koszuliński

Status: reviewreview_failed

This patch is incorrect and tests very quickly prove that. You should run tests related to the change on at least Chrome, Firefox and one of IEs before putting a branch on review.

The reason why this patch is incorrect is that Firefox handles multi-range selections, so it's not correct to remove all ranges between adding a new one.

comment:10 Changed 5 years ago by Piotrek Koszuliński

The thing that we must check is why selectRanges( ) is called with multiple ranges in http://tests.ckeditor.dev:1030/tests/plugins/enter/list, because it doesn't seem to make sense.

Note - selection.removeAllRanges() is called at the beginning of selection.selectRanges(), so if only one range was passed, then there's no chance of creating the discontiguous selection.

comment:11 Changed 5 years ago by Artur Delura

Status: review_failedassigned

comment:12 Changed 5 years ago by Piotrek Koszuliński

Resolution: fixed
Status: assignedclosed

It turns out that this issue was fixed by #12630. So there really was some reason deep under the ground.

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