Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Running autolink on text containing a mailto: link breaks

#1862

ActionView::Helpers::TextHelper#autolink does not handle certain mailto: links properly.

Assume the following html:


<a href="mailto:david@loudthinking.com">Mail me</a>

When running this html through autolink, it should not alter the html. However it is transformed to:


<a href="mailto:<a href="david@loudthinking.com">david@loudthinking.com</a>">Mail me</a>

I wrote a test to reproduce this bug and added a fix as well. See: http://github.com/mdh/rails/comm...

Reported by marek · February 3rd, 2009 @ 11:01 PM

State: resolved
Milestone: 3.x
Assigned to: nobody
Importance: none

Activity

  1. marek
  2. Amos King
    Amos King
    • Tag changed from ruby 1.8.7, actionview, auto_link, bug, edge, text_helper to ruby 1.8.7, actionview, auto_link, bug, edge, patch, text_helper

    +1

    February 4th, 2009 @ 03:12 AM

  3. Michael Koziarski
    Michael Koziarski

    That patch isn't a patch ;)

    But more fundamentally, does the other auto_link functionality support already linked text? I didn't see it looking at the tests but perhaps I'm missing it.

    If it does, then let's pull that kind of test into a new test case for both emails and urls.

    February 6th, 2009 @ 12:59 AM

  4. marek
    marek

    Oops,

    Please excuse me for screwing up. I added a proper diff file this time.

    I guess this feature is not often used.

    But yes, there is such a test in text_helper_test.rb. It's called test_auto_link_already_linked. I've expanded that test so it covers this issue too, please see patch.

    February 6th, 2009 @ 09:06 AM

  5. marek
    marek

    Very much related to this issue:

    If the html contains an img tag it gets screwed up too. I fixed that as well. See second patch file.

    February 18th, 2009 @ 09:17 AM

  6. Mislav
    Mislav

    The second issue (IMG tag) was resolved in #1523 auto_link should not linkify URLs in the middle of a tag.

    I'm attaching a patch that takes the same approach as that ticket for the first issue you reported. It's very similar to your patch, only it handles all cases of email strings found inside HTML attributes.

    March 11th, 2009 @ 10:46 AM

  7. marek
    marek

    Thanks for taking the time to look at my patch. I was not aware of #1523 auto_link should not linkify URLs in the middle of a tag. Using the same approach makes sense.

    March 11th, 2009 @ 11:09 AM

  8. Mislav
    Mislav
    • Tag changed from ruby 1.8.7, actionview, auto_link, bug, edge, patch, text_helper to actionview, auto_link, patch

    I've pushes two patches which resolve all auto_link issues in this tracker. Changes are in the "auto_link" branch of my fork

    April 17th, 2010 @ 05:37 AM

  9. Mislav
    Mislav

    I've just pushed 2-3-stable compatible version. The branch name is "auto_link_2-3-stable"

    April 17th, 2010 @ 05:53 AM

  10. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.x to 3.x

    May 4th, 2010 @ 06:48 PM

  11. Repository
    Repository
    • State changed from new to resolved

    (from [17b4fd25e4de8f05d40ccaa776e51636745aa8e8]) avoid auto_linking already linked emails; more robust detection of linked URLs

    References #1523 auto_link should not linkify URLs in the middle of a tag [#1862 Running autolink on text containing a mailto: link breaks state:resolved] [#3591 auto_link should not create a link inside a link which has the rel attribute state:resolved]

    Add test that shows how link text can contain HTML if needed:
    the trick is using block form in combination with raw.
    Let link text be automatically HTML-escaped

    [#2017 Should not html_escape auto_link block form state:resolved] http://github.com/rails/rails/commit/17b4fd25e4de8f05d40ccaa776e516...

    May 29th, 2010 @ 03:06 AM

  12. Repository
    Repository

    (from [8f0b2138ee979799092e0489f7298289c90901b9]) avoid auto_linking already linked emails; more robust detection of linked URLs

    References #1523 auto_link should not linkify URLs in the middle of a tag [#1862 Running autolink on text containing a mailto: link breaks state:resolved] [#3591 auto_link should not create a link inside a link which has the rel attribute state:resolved]

    Add test that shows how link text can contain HTML if needed:
    the trick is using block form in combination with raw.
    Let link text be automatically HTML-escaped

    [#2017 Should not html_escape auto_link block form state:resolved] http://github.com/rails/rails/commit/8f0b2138ee979799092e0489f72982...

    May 29th, 2010 @ 03:06 AM