[CKEditor Dev] #10438: [FF] No selection put in editable on setData
CKEditor
------------------------------+--------------------------
Reporter: Reinmar | Owner:
Type: Bug | Status: new
Priority: Normal | Milestone: CKEditor 4.2
Component: Core : Selection | Version:
Keywords: |
------------------------------+--------------------------
TODO
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438>
CKEditor <http://ckeditor.com/>
The text editor for the Internet
CKEditor
Reporter: Reinmar | Owner:
Type: Bug | Status: confirmed
Priority: Normal | Milestone: CKEditor 4.2
Component: Core : Selection | Version:
-------------------------------+--------------------------
Changes (by Reinmar):
* status: new => confirmed
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438#comment:1>
CKEditor
Reporter: Reinmar | Owner: Reinmar
Type: Bug | Status: assigned
Priority: Normal | Milestone: CKEditor 4.2
Component: Core : Selection | Version:
-------------------------------+--------------------------
Changes (by Reinmar):
* owner: => Reinmar
* status: confirmed => assigned
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438#comment:2>
CKEditor
-------------------------------+--------------------------
Reporter: Reinmar | Owner: Reinmar
Type: Bug | Status: assigned
Component: Core : Selection | Version:
-------------------------------+--------------------------
Description changed by Reinmar:
Old description:
> TODO
New description:
Attached two samples.
TC:
1. Open framed_focus.html sample.
2. Open console.
3. Execute `setData();`.
4. Quickly focus editor and wait.
5. After data will be set verify:
* Whether caret blinks.
* Whether `EDITOR#BLUR` is logged when you click outside editor.
* Whether you can type and whether after typing additional paragraph
is not created (what means that selection was placed outside editable
place.
--
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438#comment:3>
CKEditor
-------------------------------+--------------------------
Reporter: Reinmar | Owner: Reinmar
Type: Bug | Status: assigned
Component: Core : Selection | Version:
-------------------------------+--------------------------
Comment (by Reinmar):
Pushed t/10438 on tests and t/10438 on dev with prototype of a patch.
Tests are fixed on FF and IE9 and they pass on Webkit on master. I haven't
yet tested whether IE8, IE10 and Opera also require this fix.
Manual tests on framed_focus.html also pass.
Manual tests on inline_focus.html does not fully pass - selection is not
placed in editable place on FF and IE (Chrome is ok). On IE9 additionally,
selection is placed at the end of editable, not at the beginning.
This makes me thing that perhaps we should promote these fixes to some
editor's method called on every setData. Perhaps the original fix for
Webkit also does not have to be in selection construtor. I've found that
it was proposed for these issues:
{{{
<li><a href="http://dev.ckeditor.com/ticket/6192">#6192</a> : [WebKit]
Inserting smileys was not working because of editor focus issues.</li>
<li><a href="http://dev.ckeditor.com/ticket/6178">#6178</a> : [WebKit]
Inserting elements by code was failing if the editor didn't receive the
focus first.</li>
<li><a href="http://dev.ckeditor.com/ticket/6179">#6179</a> : [WebKit] The
Image dialog was not working if the editor didn't receive the focus
first.</li>
}}}
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438#comment:4>
CKEditor
-------------------------------+--------------------------
Reporter: Reinmar | Owner: Reinmar
Type: Bug | Status: assigned
Component: Core : Selection | Version:
-------------------------------+--------------------------
Comment (by Reinmar):
There will be another advantage of promoting this fix higher. We will be
able to make the initial caret location configurable and I have seen such
requests.
Although, since we are short on time, we may work on extracting this patch
later, because that would require some research and tests.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438#comment:5>
CKEditor
-------------------------------+--------------------------
Reporter: Reinmar | Owner: Reinmar
Type: Bug | Status: assigned
Component: Core : Selection | Version:
-------------------------------+--------------------------
Comment (by Reinmar):
[FF] Tickets to test against this: #4472, #5021, #3864, #5781.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438#comment:6>
CKEditor
-------------------------------+--------------------------
Reporter: Reinmar | Owner: Reinmar
Type: Bug | Status: assigned
Component: Core : Selection | Version:
-------------------------------+--------------------------
Comment (by Reinmar):
I pushed few more commits to tests and dev making all browsers pass them.
* IE7/8 - very similar fix to that for IE9.
* Opera - it says: 'everything is fine, really! selection and active
element are fine!'... yeah - right. I haven't found a way to discover that
selection is broken, so I just added tests to ignored.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438#comment:7>
CKEditor
-------------------------------+--------------------------
Reporter: Reinmar | Owner: Reinmar
Type: Bug | Status: assigned
Component: Core : Selection | Version:
-------------------------------+--------------------------
Comment (by Reinmar):
It turns out that we have to improve this patch and promote for all
`setData` calls (also initial one) earlier than I thought. As #10439
proves, we need to have consistent initial selection (before first focus,
after focus&setData, after switching from source mode and in all other
cases) in all browsers.
In cases when we do not need selection locking (FF and Chrome with framed
editor) it will be as simple as it is now - on editor load we need to set
selection in the right place (`range.moveToElementEditStart( this.root,
true/false )`). In other cases I think that we can mock initial locked
selection, as the editor was focused before. Although... brace yourself -
critical core changes are coming.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438#comment:8>
CKEditor
-------------------------------+--------------------------
Reporter: Reinmar | Owner: Reinmar
Priority: Normal | Milestone: CKEditor 4.2
Component: Core : Selection | Version:
-------------------------------+--------------------------
Changes (by Reinmar):
* status: assigned => review
Comment:
I pushed simplified version of t/10438.
Tests:
* FF latest - 100%
* Chrome - 100%
* FF 3.6 - regression: http://ckeditor4.t/dt/core/selection/editor.html -
"test selection on initial focus - ensure new doc" - but do we support
3.6?
* IE9 - 100%
* IE8 - 100% but I got alert `'Stack overflow at line: 0'` in
http://ckeditor4.t/dt/core/htmldataprocessor.html which... does not test
selection at all. Although, it loads data, so perhaps this is it. I'll
debug this if you can confirm alert.
Note: Tests created for this ticket contains some events logging - I left
this intentionally - they help while testing.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438#comment:9>
CKEditor
-------------------------------+--------------------------
Reporter: Reinmar | Owner: Reinmar
Type: Bug | Status: review
Component: Core : Selection | Version:
-------------------------------+--------------------------
Description changed by Reinmar:
Old description:
> Attached two samples.
New description:
Attached two samples.
Note: selection issues (3rd point) in inline editor are not a subject of
this ticket. The most important part is fixing blur problem (2nd point),
selection in framed editor is fixed because it was convenient.
--
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438#comment:10>
CKEditor
-------------------------------+--------------------------
Reporter: Reinmar | Owner: Reinmar
Type: Bug | Status: review
Component: Core : Selection | Version:
-------------------------------+--------------------------
Comment (by Reinmar):
I tested FF latest for #4472, #5021, #3864, #5781 which could become
regressions after git:cc72d842 (Commit: "Removed unncessary hack."), but
everything is fine.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438#comment:11>
CKEditor
-------------------------------+---------------------------
Reporter: Reinmar | Owner: Reinmar
Type: Bug | Status: review_passed
Priority: Normal | Milestone: CKEditor 4.2
Component: Core : Selection | Version:
-------------------------------+---------------------------
Changes (by fredck):
* status: review => review_passed
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438#comment:12>
CKEditor
-------------------------------+---------------------------
Reporter: Reinmar | Owner: Reinmar
Type: Bug | Status: review_passed
Component: Core : Selection | Version:
-------------------------------+---------------------------
Comment (by Reinmar):
I tried to debug that IE8's stack overflow alert. I reduced the TC to the
absolute minimum:
{{{
<!DOCTYPE html>
<html>
<head>
<title>CKEDITOR.htmlDataProcessor</title>
<meta name="cktester-tags" content="editor,unit">
<script src="../../cktester/cktester.js"></script>
<script>
CKTESTER.editor = {
config: {
enterMode: CKEDITOR.ENTER_BR
}
};
CKTESTER.test( {
'ie, we will never forget what you did to us': function()
{
this.editor.focus();
this.editorBot.setData( '', function() {
assert.isTrue( true );
} );
}
} );
</script>
</head>
<body></body>
</html>
}}}
Still, alert is opened.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438#comment:13>
CKEditor
-------------------------------+---------------------------
Reporter: Reinmar | Owner: Reinmar
Type: Bug | Status: review_passed
Component: Core : Selection | Version:
-------------------------------+---------------------------
Comment (by Reinmar):
OK. I pushed additional fix for IEs preventing from infinitive recursion
between this new selection fix and fix for #6966. It would be cool to find
a better solution, but for now I'm closing this issue, because it's
blocking us.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438#comment:14>
CKEditor
-------------------------------+--------------------------
Reporter: Reinmar | Owner: Reinmar
Type: Bug | Status: closed
Priority: Normal | Milestone: CKEditor 4.2
Component: Core : Selection | Version:
-------------------------------+--------------------------
Changes (by Reinmar):
* status: review_passed => closed
* resolution: => fixed
Comment:
Fixed on major with git:b83e32f on dev and c84d41f on tests.
This is the commit which would be cool to enhance: git:260015a1.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10438#comment:15>