Opened 4 years ago

Last modified 3 years ago

#13593 review Bug

[iOS, Android, WP] Fire editor#selectionChange on selection changes caused by touch events

Reported by: Piotrek Koszuliński Owned by: Olek Nowodziński
Priority: Normal Milestone:
Component: General Version:
Keywords: iOS Android NiceToHave Cc:

Description (last modified by Piotrek Koszuliński)

Currently editor#selectionChange is based on:

  • native selectionchange,
  • keyboard events,
  • mouse events.

Therefore, CKEditor does not know about selection changes done on touch devices if browser does not implement selectionchange itself. It may also happen that we have some bugs in this system.

Lack of editor#selectionChange and checkSelectionChange() calls result in various issues like wrong selection being locked on focus (we store a reference to the last selection) and lack of updates in commands states. I think that related tickets may be:

Change History (24)

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

Status: newconfirmed
Summary: Fire editor#selectionChange on selection changes caused by touch events[iOS, Android] Fire editor#selectionChange on selection changes caused by touch events

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

Milestone: CKEditor 4.5.3CKEditor 4.5.4

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

Description: modified (diff)
Summary: [iOS, Android] Fire editor#selectionChange on selection changes caused by touch events[iOS, Android, WP] Fire editor#selectionChange on selection changes caused by touch events

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

Description: modified (diff)

comment:5 Changed 4 years ago by Szymon Cofalik

Owner: set to Szymon Cofalik
Status: confirmedassigned

comment:6 Changed 4 years ago by Olek Nowodziński

Owner: changed from Szymon Cofalik to Olek Nowodziński

comment:7 Changed 4 years ago by Olek Nowodziński

Status: assignedreview

The solution pushed to branch:t/13593. It fixes #13522 and #13573 straight away.

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

Status: reviewreview_failed

Unfortunately on iOS making a non-empty selection does not trigger selectionChange. I wonder if iOS fires any event when you modify the selection ;/

comment:9 Changed 4 years ago by Olek Nowodziński

Status: review_failedreview

It looks like iOS fires selectionchange on document instead of editable. The additional listener fixes all selection issues in iOS, as well as in Android.

branch:t/13593b

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

Status: reviewreview_failed

There's something wrong with this branch. I have one red test here http://tests.ckeditor.dev:1030/tests/core/editable/status (on Chrome and Edge at least). Plus, I think that there's some inf loop or something in some other tests as they crashed.

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

I've just checked that the selectionchange listener that we had isn't necessary at all as on every browser I checked the event is fired on document only. Well, the only reason that it must exist is that our code fires custom selectionchange event on editable once it changed the selection. So we can easily have two listeners and there will be no overlapping at all. Uuuunless... One day the selectionchange event will start to fire on the editing host as well but that should not happen:

When the selection is dissociated with its range, associated with a new range or the associated range's boundary point is mutated either by the user or the content script, the user agent must queue a task ([HTML5]) to fire an event with the name selectionchange, which does not bubble and is not cancelable, at the document associated with the selection.

source: http://w3c.github.io/selection-api/#selectionchange-event

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

Milestone: CKEditor 4.5.4CKEditor 4.5.5
Owner: Olek Nowodziński deleted
Status: review_failedconfirmed

comment:13 Changed 4 years ago by Olek Nowodziński

Pushed experimental branch:t/13593c based on branch:zws. It looks like the new FC system solved the problem with loops (doc#selectionchange->remove FC from DOM->doc#selectionchange). It requires further testing and research though.

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

!! Thanks !!

comment:15 Changed 4 years ago by Olek Nowodziński

Development of the new FC strategy can be tracked in #13816. Rebased branch:t/13593c on branch:t/13816.

comment:16 Changed 4 years ago by Olek Nowodziński

Owner: set to Olek Nowodziński
Status: confirmedreview

comment:17 Changed 4 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.5CKEditor 4.5.6

comment:18 Changed 4 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.6CKEditor 4.5.7

comment:19 Changed 4 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.7CKEditor 4.5.8

comment:20 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.8CKEditor 4.5.9

comment:21 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.9CKEditor 4.5.10

comment:22 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.10CKEditor 4.5.11

Moving tickets to the next milestone.

comment:23 Changed 3 years ago by Marek Lewandowski

Milestone: CKEditor 4.5.11CKEditor 4.6.1

comment:24 Changed 3 years ago by Marek Lewandowski

Keywords: NiceToHave added
Milestone: CKEditor 4.6.1

We won't be able to contain it soon, moving to nice to have list.

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