Opened 15 years ago
Closed 15 years ago
#6192 closed Bug (fixed)
Webkit: editor.focus() fails if no selection ranges exist
| Reported by: | Jude Allred | Owned by: | Frederico Caldeira Knabben |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 3.4.1 |
| Component: | General | Version: | 3.4.1 |
| Keywords: | WebKit | Cc: |
Description (last modified by )
This causes insertion into the editor to fail.
Repro steps:
- use Chrome
- go to ckeditor.com/demo
- without focusing the editor, attempt to insert a smiley
- smiley insertion fails. [expected] a smily is inserted.
Ditto for tables, attachments, and all other insertables.
I've written and tested a fix. It's attached as an hg patch file.
Attachments (5)
Change History (21)
Changed 15 years ago by
| Attachment: | webkit focus.patch added |
|---|
comment:1 Changed 15 years ago by
comment:2 Changed 15 years ago by
| Keywords: | Webkit added |
|---|---|
| Status: | new → confirmed |
| Version: | → 3.4.1 (SVN - trunk) |
Bug confirmed on Chrome build 6.0.495.0
comment:3 Changed 15 years ago by
| Milestone: | → CKEditor 3.4.1 |
|---|
comment:4 Changed 15 years ago by
| Keywords: | HasPatch added |
|---|
comment:5 Changed 15 years ago by
| Owner: | set to Tobiasz Cudnik |
|---|---|
| Status: | confirmed → assigned |
Changed 15 years ago by
| Attachment: | 6192.patch added |
|---|
comment:6 Changed 15 years ago by
| Status: | assigned → review |
|---|
comment:7 Changed 15 years ago by
| Description: | modified (diff) |
|---|
comment:8 Changed 15 years ago by
| Status: | review → review_failed |
|---|
Some notes:
- Style:
BAD --> if ( editor.getSelection().getRanges().length === 0 ) GOOD -> if ( !editor.getSelection().getRanges().length )
- Considering that the selection is being retrieved in the if(), it should be instead cached in a variable to be reused inside of it.
- Strange stuff:
// What's this??? $( doc.$.body ).children()[ 0 ]
Considering the third issue, which should definitely through an error, I'm assuming that if block has never been entered, because I have no errors. So, what's the TC to make it enter there?
Other than the mysterious functions, what if body has no children?
comment:9 Changed 15 years ago by
| Status: | review_failed → review |
|---|
It seems this issue can be fixed by only focusing HTML element once.
I've split part of the patch (an IF block) to the #6153. All 3 issues of previous patch have been fixed.
Changed 15 years ago by
| Attachment: | 6192_2.patch added |
|---|
comment:11 Changed 15 years ago by
| Status: | review → review_failed |
|---|
- There is no need to initialize the wasFocused variable.
- When loading the editor (or switching back from source) with long contents, if you scroll down with the mousewhell and click in a paragraph at the bottom, the caret will be placed there, but the contents will scroll up.
- Let's have a reduced TC for this WebKit issue reported at their side as well.
Changed 15 years ago by
| Attachment: | 6192_3.patch added |
|---|
comment:12 Changed 15 years ago by
| Status: | review_failed → review |
|---|
Third patch fixes scrolling issue with similar way pasting does.
Unfortunately i wasn't able to prepare reduced TC for this issue, as contenteditable iframe is always focused properly, including when iframe element is generated dynamically. It seems this issue relays on our complex focus logic.
comment:13 Changed 15 years ago by
| Keywords: | WebKit added; Webkit HasPatch removed |
|---|---|
| Status: | review → review_failed |
Not been able to have a reduced tc for this browser issue represents a failure on understanding the root of the problem, not being able to solve it properly.
I've made some investigation on this, opening a WebKit bug for it:
https://bugs.webkit.org/show_bug.cgi?id=45686
Based on that, I've found out that the fix here was taking the wrong path. I'll provide something different.
Changed 15 years ago by
| Attachment: | 6192_4.patch added |
|---|
comment:14 Changed 15 years ago by
| Owner: | changed from Tobiasz Cudnik to Frederico Caldeira Knabben |
|---|---|
| Status: | review_failed → review |
comment:15 Changed 15 years ago by
| Status: | review → review_passed |
|---|
Your patch is much better than mine for this issue, although we still need the previous one to fix #6153.
comment:16 Changed 15 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review_passed → closed |
Fixed with [5882].

Note: The patch is meant to be applied to wysiwygarea/plugin.js