Autoembed's rule which checks whether link's href equals links content should take encoding into consideration

[Piotrek Koszuliński]

1. Open ​http://tests.ckeditor.dev:1030/tests/plugins/autoembed/manual/autoembed
2. Paste: ​https://www.youtube.com/watch?v=GUl9_5kK9ts&x=1

Result: it does not trigger autoembedding. That's because of the "&" character which is encoded in text node, but it isn't encoded in link.attributes.href.

[Szymon Cofalik]

Fixed on ​branch:t/13420.

I changed how links are decided to be autoembedded or not. The requirement that has not been met in bugged version is that the link has to have same href and inner text. So I changed the regexp to check if such link was pasted.

Previously, it was done through htmlParser which messes too much with what was pasted. I tried to toy with encodeURIComponent() and try to tweak previous solution but the code got too messy and there was a few cases to think about. Now it's clean and works fine.

[Olek Nowodziński]

I rebased the branch on master and pushed a commit ​git:5aea655 with a test which refers to #13419. Unfortunately, since #13419, we should not assume that for href always corresponds with innerHTML of a link for true embeddables. There got to be smarter than that.

[Olek Nowodziński]

Idea: what if we used decodeURI() to normalise href and then compare to innerHTML?

[Szymon Cofalik]

I am not sure how it will work with links that has already encoded chars in their text. Will have to check.

[Szymon Cofalik]

Fixed on ​branch:t/13420.

I also pushed ​branch:t/13420b with a bit more expanded solution. This solution passes some more complicated tests. It also uses htmlentity decoding to change signs like & into & when matching if href equals contents.

Theoretically this may be a better solution, but those test cases are not reproducible manually, even if I write <a href="https://foo.bar/g/200/313?foo=%22æåãĂĄ&quot;">https://foo.bar/g/200/313?foo=&quot;æåãĂĄ"</a> in source, then copy this link and paste into editor - browser or CKE (not sure which one) takes care of those entities before we get the data in autoembed plugin. So these extra tests are only theoretical cases. It might be better to use this fix though if this behavior change somewhere in the future.

[Olek Nowodziński]

​​branch:t/13420 is enough right now and I'm OK to close it.

[Olek Nowodziński]

Merged ​git:2b30328 into master. Good job!