[FF&IE] No selection put in editable on setData

[Piotrek Koszuliński]

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.

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.

[Piotrek Koszuliński]

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>

[Piotrek Koszuliński]

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.

[Piotrek Koszuliński]

[FF] Tickets to test against this: #4472, #5021, #3864, #5781.

[Piotrek Koszuliński]

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.

[Piotrek Koszuliński]

It turns out that we have to improve this patch and use 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.

[Piotrek Koszuliński]

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.

[Piotrek Koszuliński]

I tested FF latest for #4472, #5021, #3864, #5781 which could become regressions after ​git:cc72d842 (Commit: "Removed unncessary hack."), but everything is fine.

[Piotrek Koszuliński]

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.

Updates:

• IE8 doesn't allow me to place break point in TC file or it misses it.
• I overwrote alert function, but apparently IE calls it internally.
• It's not possible to catch that error (cause there's no error) in dev or test code.
• Surprisingly, IE9 also notifies about some problems:

  SCRIPT28: Stack overflow 
  document.js, line 220 char 3
  SCRIPT2343: Stack overflow in line: 220 
  

• There really is some kind of inf recursion. It's getting hotter here!

[Piotrek Koszuliński]

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.

[Piotrek Koszuliński]

Fixed on major with ​git:b83e32f on dev and c84d41f on tests.

This is the commit which would be cool to enhance: ​git:260015a1.