Editable#insertText loses $ characters

[Piotrek Koszuliński]

Based on: ​http://stackoverflow.com/questions/16161826/ckeditor-issue-with-inserttext

Call editor.insertText( '$$' ) in styled text (bold, italic, etc). One of the dollars will be lost.

The reason is this line: ​https://github.com/ckeditor/ckeditor-dev/blob/master/core/editable.js#L1616

Dollar characters make some troubles when used in replace string.

[Jakub Ś]

Problem can be reproduced in all browsers form CKEditor 4.0 beta (works in 3.x branch).

Below can be executed in browser console:

CKEDITOR.instances.editor1.insertText( '$$' ); 

[Frederico Caldeira Knabben]

Pushed t/10367 in cksource and tests.

[Piotrek Koszuliński]

> git co t/10367
error: pathspec 't/10367' did not match any file(s) known to git.

Have you pushed test branch?

[Piotrek Koszuliński]

BTW. the patch cannot be correct:

> 'a$bcde'.replace( /$/g, '$$$$' )
> "a$bcde$$"

Dollar character needs to be escaped of course.

Also, I haven't seen tests yet, but they should cover all special replacement patterns: $$, $&, $, $', $n (where n is 0-99)`.

In case of more issues, I'd propose to fix this by using split, splice and join.

[Frederico Caldeira Knabben]

Well.. don't know what to say. I was sure I had a test for it, but it got lost. My mistake, for sure.

In any case, the review is definitely correct.

I've just pushed it again to t/10367 in cksource and tests.

[Olek Nowodziński]

The fix fails on IE7, because:

alert( 'foo'.replace( 'foo', '$$$$' ) );

• regular browser: $$
• IE7: $$$$

[Olek Nowodziński]

Rebased both branches on latest master.

• Added MT for the ticket.
• Proposed split&join solution instead of regex replace.

[Frederico Caldeira Knabben]

I don't think we need this mt as it is a plain duplication of the introduced automated tests.

As for the rest, we're definitely ok.

[Olek Nowodziński]

Merged fix into master, dev ​git:a4c8d0f133, tests 6cc696129ed8fc.