ACF blocks tabSpaces

[Jakub Ś]

Seems Allowed Content Filter blocks ​http://docs.ckeditor.com/#!/api/CKEDITOR.config-cfg-tabSpaces. Tab key causes focus on next element instead of inserting spaces into content area.

The below will not work without allowedContent: true

var editor = CKEDITOR.replace( 'editor1', {
	tabSpaces : 4,
	//allowedContent: true				
} );	 

[Mike Burgh]

Following

[Piotrek Koszuliński]

Duplicate: #10271

[Piotrek Koszuliński]

ACF does not pass <span data-cke-marker="1"> element which editable#insertHtml uses (​https://github.com/ckeditor/ckeditor-dev/blob/master/core/editable.js#L1127) to surround spaces when processing data.

I found that ACF also does not pass bookmarks (<span data-cke-bookmark="1" style="display:none;">&nbsp;</span>) and who knows how many other elements that we may need to sometimes insert to the editor.

So a basic solution would be to extend ACF with this kind of rule (this should be placed in filter constructor, next to default rule for p/br/div):

this.allow( {
	span: {
		match: function( el ) {
			return Object.keys( el.attributes ).some( function( attr ) {
				return attr.indexOf( 'data-cke-' ) > -1;
			} );
		},
		styles: 'display'
	}
}, 'default', 1 );

(Note: we don't need to specify data-cke-* attributes, because they all are passed by default)

However:

• We will allow these spans in data outputted from editor (on the other hand we already allow data-cke-* attributes on all elements). So... if you agree that we shouldn't allow them for both - output and input, then this rule has to be implemented internally, inside filter. Note that CKEDITOR.filter#applyTo has 2nd argument which tells in which direction data is processed. The correct place for this rule will be around these lines - ​https://github.com/ckeditor/ckeditor-dev/blob/master/core/filter.js#L281-L288

This approach has another advantage - we will not expose in filter.allowedContent rule which is fully internal.

• We haven't caught this issue before, because tests for editable#insertXXX are done with disabled ACF. I think that this case proved that we should enable ACF in those tests by setting minimum config.allowedContent that they need.

[Olek Nowodziński]

I'm for keeping stuff like this internally and as simple as possible. Thus I created branch ​t/10260 (+tests) with a fix for the issue inside of CKEDITOR.filter#applyTo.

[Piotrek Koszuliński]

Fixes also #10302.

[Piotrek Koszuliński]

I rebased and pushed both branches. Additionally - I pushed more tests and it turned out that t/10260@dev does not pass one of test cases. Filter should not accept CKE's stuff in output.

[Olek Nowodziński]

Updated both t/10260@dev and t/10260@tests.

I restricted the "holy cow mode" for data-cke-* spans to dataProcessor.toHtml() only as we don't want them on output. I also fixed some new test cases since dataProcessor.toDataFormat() avoids filtering, so switching tests to basic filter.applyTo() was necessary.

[Olek Nowodziński]

Commits merged into master: ​git:d84ac37780390b6b, 79acdab8adf1360 (tests).

[Piotrek Koszuliński]

Please, verify also if this patch fixes #10302 too.

[Olek Nowodziński]

This patch also fixes #10302.