wordpress-mobile / AztecEditor-Android

A reusable native Android rich text editor component.
Mozilla Public License 2.0
675 stars 112 forks source link

Proper handling of email, schemeless URLs #708

Closed hypest closed 5 years ago

hypest commented 5 years ago

Fix #460

This PR mirrors the behavior from Calypso to handle email links and links that need the scheme added.

Test 1

  1. Open the demo app and tap on the toolbar to insert a link
  2. In the URL field type: name@company.com, copy the same to the Link text field and tap OK
  3. Switch to html and notice that the href points to: mailto:name@company.com (prepended with mailto:)

Test 2

  1. Open the demo app and tap on the toolbar to insert a link
  2. In the URL field type: #internalanchor, copy the same to the Link text field and tap OK
  3. Switch to html and notice that the href points to: #internalanchor (no http:// added)

Test 3

  1. Open the demo app and tap on the toolbar to insert a link
  2. In the URL field type: wordpress.org, copy the same to the Link text field and tap OK
  3. Switch to html and notice that the href points to: http://wordpress.org (there's an added http://)
daniloercoli commented 5 years ago

We should also escape the double quote character (" ) when inserted in links, otherwise we're breaking the resulting HTML link. screen shot 2018-06-11 at 16 38 03

daniloercoli commented 5 years ago

For reference pasting here the link to email tests taken from chromium.

I've also found an interesting post on stackoverflow that could help with Regexps if we decide to investigate this further.

hypest commented 5 years ago

We should also escape the double quote character (" ) when inserted in links, otherwise we're breaking the resulting HTML link.

I added code (with 19c40b3) to encode the link text @daniloercoli . Let me know if that's what you had in mind.

By the way, as it currently is, the email regex won't match when there's a doublequote in the link text. How can I reproduce the case in the screenshot you shared above?

For reference pasting here the link to email tests taken from chromium.

I've also found an interesting post on stackoverflow that could help with Regexps if we decide to investigate this further.

Aha, thanks for looking into this! I feel though that an extensive support like that is outside the scope of this PR. Can we do it in a separate PR? If OK with you, can you open a ticket with the support you'd like to see implemented? Thanks!

daniloercoli commented 5 years ago

Aha, thanks for looking into this! I feel though that an extensive support like that i outside the scope if this PR. Can we do it in a separate PR? If OK with you, can you open a ticket with the support you'd like to see implemented? Thanks!

I agree and will open a new ticket.

daniloercoli commented 5 years ago

By the way, as it currently is, the email regex won't match when there's a doublequote in the link text. How can I reproduce the case in the screenshot you shared above?

If you try by inserting dan"@text.com you will see there are 2 errors:

daniloercoli commented 5 years ago

:shipit:

hypest commented 5 years ago

Thanks for the merge and for the new ticket @daniloercoli . We can move any regex discussion there I guess, cool?