Opened 2 years ago

Closed 9 months ago

#13446 closed Bug (fixed)

[Chrome] It is possible to type in unfocused inline editor

Reported by: Jakub Ś Owned by: Tomasz Jakut
Priority: Normal Milestone: CKEditor 4.6.2
Component: General Version: 4.0 Beta
Keywords: Chrome Support Cc:

Description

I have managed to reproduce it only in Chrome from CKEditor 4.0 beta.

  1. Go to massive inline creation sample
  2. Click into last inline editor (that small one at the very bottom)
  3. Press tab - Focus goes to link
  4. Star typing

Result - text is being typed into editor.

Change History (19)

comment:1 Changed 2 years ago by Jakub Ś

Keywords: Support added
Status: newconfirmed

comment:2 Changed 2 years ago by Jonathan

I have also verified the issue exists in version 4.4.7 with the div editing area plugin.

comment:3 Changed 23 months ago by Marek Lewandowski

Milestone: CKEditor 4.5.6

comment:4 Changed 23 months ago by Tomasz Jakut

Owner: set to Tomasz Jakut
Status: confirmedassigned

comment:5 Changed 23 months ago by Tomasz Jakut

Status: assignedreview

The only working fix is removing the entire selection after focusing element outside the editor. Pushed branch:t/13446.

comment:6 Changed 22 months ago by Olek Nowodziński

Status: reviewreview_failed
  1. I ran tests in Chrome and got over a hundred of Uncaught TypeError: Cannot read property 'getNative' of null errors coming from https://github.com/cksource/ckeditor-dev/blob/t/13446/core/focusmanager.js#L161 in many different tests.
  1. A selection cached in https://github.com/cksource/ckeditor-dev/blob/t/13446/core/focusmanager.js#L153 may be outdated when doBlur() is called in the timout https://github.com/cksource/ckeditor-dev/blob/t/13446/core/focusmanager.js#L178-L181. It's unsafe and may be a source of problems in the future.
  1. https://github.com/cksource/ckeditor-dev/blob/t/13446/core/focusmanager.js#L162 – shouldn't it be editor.window? What would happen to a classic editor in such case?
  1. I see no tests for classic editor (wysiwygarea–based).

comment:7 Changed 22 months ago by Marek Lewandowski

Milestone: CKEditor 4.5.6CKEditor 4.5.7

comment:8 Changed 22 months ago by Tomasz Jakut

Status: review_failedreview

1, 2. I changed fix to detect inline mode in Chrome, not the existing selection. It seems to fix errors and removes the need of caching selection.

  1. Yes, it should be. I've changed that. Classic editor should be not touched (changes for 1. ensures that).
  2. I added test for classic editor to check if fix is not touching it (Chrome doesn't clean up selection in classic editor too, but it doesn't cause any errors, because the focus is moved entirely outside editor's window on blur - therefore fix leaves it as it is).

Pushed branch:t/13446.

comment:9 Changed 20 months ago by Marek Lewandowski

Milestone: CKEditor 4.5.7CKEditor 4.5.8

comment:10 Changed 18 months ago by Marek Lewandowski

Milestone: CKEditor 4.5.8CKEditor 4.5.9

comment:11 Changed 17 months ago by Marek Lewandowski

Milestone: CKEditor 4.5.9CKEditor 4.5.10

comment:12 Changed 15 months ago by Marek Lewandowski

Milestone: CKEditor 4.5.10CKEditor 4.5.11

Moving tickets to the next milestone.

comment:13 Changed 14 months ago by Marek Lewandowski

Milestone: CKEditor 4.5.11CKEditor 4.6.1

comment:14 Changed 10 months 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:15 Changed 9 months ago by kkrzton

Status: reviewreview_failed

Rebased it on current master and updated manual test version tag.

This solution looks like a good approach to solve this issue. However, one unit test is failing http://tests.ckeditor.dev:1030/tests/core/editor/title with Uncaught TypeError: Cannot read property 'hasAscendant' of null .

comment:16 Changed 9 months ago by Tomasz Jakut

Status: review_failedreview

I've updated code on branch:t/13446.

The issue with the test is caused by the fact that handling for editor's blur is fired asynchronously. The invocation of insertText in loop, for all editors cause the blur handler to be fired after focusing the next editor, eventually clearing the selection in this editor's instance – if user is using Chrome and that editors are inline ones. IMO it's a situation very unlike to happen in the real world so fixing just the test should suffice.

I've also simplified checking for the inline editor in focusmanager.js.

comment:17 Changed 9 months ago by kkrzton

Summary: It is possible to type in unfocused inline editor[Chrome] It is possible to type in unfocused inline editor

comment:18 Changed 9 months ago by kkrzton

Status: reviewreview_passed

Fixing tests is fair enough IMO. LGTM!

comment:19 Changed 9 months ago by kkrzton

Resolution: fixed
Status: review_passedclosed

Merged to master with 8437204.

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