Opened 6 years ago

Closed 5 years ago

#10906 closed Bug (fixed)

[IE11+] Editable is not being focused when clicking outside body

Reported by: Piotrek Koszuliński Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.3
Component: General Version:
Keywords: IE11 Cc:

Description

In #10612 we disabled old hack because it uses old Selection&Range APIs. It needs to be brought back.

Change History (14)

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

Status: newconfirmed

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

Owner: set to Piotrek Koszuliński
Status: confirmedreview

Pushed t/10906 with a proposal. Branch is based on t/10612b.

It changes the behaviour a little bit, but it's extremely easy.

In previous version selection has been taken from the old API which returned selection updated after the click. For example if click was on the right, selection was moved to right, if in the middle, selection was placed in text somewhere in the middle of it and so on.

The standard API (the only available on IE11) does not provide such selection - after click it is empty.

My proposal is to just focus editable, which will cause restoring the selection before click. This is plain simple solution, which makes the behaviour a little bit less natural, but... it's simple.

We can keep the old code for IEs<11, but I think that we can take this opportunity to simplify code.

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

Summary: [IE11+] Editable is not being focused when clicking outside of body[IE11+] Editable is not being focused when clicking outside body

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

I think that we could just add IE to this hack: #10945 (introduced in 4.2.2 for Chrome).

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

Owner: changed from Piotrek Koszuliński to Olek Nowodziński
Status: reviewassigned

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

Status: assignedreview

Created branch t/10906b that re-uses solution from #10945 for IE11. It works but with a slight timeout.

comment:7 Changed 6 years ago by Piotrek Koszuliński

Status: reviewreview_failed

I meant moving this entire hack: https://github.com/ckeditor/ckeditor-dev/blob/major/core/selection.js#L554-L582 to the wysiwygarea, because as I checked with https://github.com/cksource/ckeditor-dev/commits/t/10906, the Chrome's patch is enough for IEs too.

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

Owner: changed from Olek Nowodziński to Piotrek Koszuliński
Status: review_failedassigned

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

Status: assignedreview

I pushed t/10906c with patch which moves the hack to wysiwygarea and simplfies it to simple editable.focus(), which works on IE11 too.

The only downside of this solution is that on IEs<11 it works a little bit worse, because caret is not placed right above the cursor, but always in the place where it was before. I don't think it is a big deal though, because users usually click in a random location when they want to focus empty editor.

comment:10 in reply to:  9 Changed 5 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

Replying to Reinmar:

The only downside of this solution is that on IEs<11 it works a little bit worse, because caret is not placed right above the cursor, but always in the place where it was before. I don't think it is a big deal though, because users usually click in a random location when they want to focus empty editor.

Well, it IS kinda important issue and in fact we had to deal with this in the past.

We can move on with the current solution in any case, but please open a new ticket for the remaining issue, pointing to here so we don't forget about it.

Btw, some extra empty lines have been added to the code wrongly. Please have then fixed before majorising.

comment:11 Changed 5 years ago by Frederico Caldeira Knabben

Status: review_passedreview_failed

The current solution broke the behavior we had on previous IEs, so we should make this a IE11 only issue, not generalize it.

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

Status: review_failedreview

Ok, so I got back to t/10906b. Now only IE11+ are covered.

Also, I created a follow up ticket: #11060.

comment:13 Changed 5 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

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

Resolution: fixed
Status: review_passedclosed

Fixed on major with git:4db3dbb.

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