Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(jqLite): IE8 no longer replaces link text content with mailto value #1982

Closed

Conversation

gonzaloruizdevilla
Copy link
Contributor

fixes #1949

IE bug: http://webbugtrack.blogspot.com.es/2008/07/bug-140-changing-mailto-links-error-in.html

Only happens when the link has only text nodes. To prevent IE from replacing the text content, the text nodes must be detached before setting the href value and reattached afterwards.

I tried to make a unit test, but I'm not able to reproduce the bug inside the test. I tried something like:

var link = angular.element('<a href="">text</a>');
link.attr("href", "mailto:name@domain.com");
expect(link.text()).toBe("text");

but with this the test doesn't fail before applying the change.

@petebacondarwin
Copy link
Contributor

Perhaps you need to trigger a reflow of the DOM by asking for the
scrollWidth or something on that node?

On 10 February 2013 12:58, Gonzalo Ruiz de Villa
notifications@github.comwrote:

fixes #1949 #1949

IE bug:
http://webbugtrack.blogspot.com.es/2008/07/bug-140-changing-mailto-links-error-in.html

Only happens when the link has only text nodes. To prevent IE from
replacing the text content, the text nodes must be detached before setting
the href value and reattached afterwards.

I tried to make a unit test, but I'm not able to reproduce the bug inside
the test. I tried something like:

var link = angular.element('text');link.attr("href", "mailto:name@domain.com");expect(link.text()).toBe("text");

but with this the test doesn't fail before applying the change.

You can merge this Pull Request by running

git pull https://github.com/gonzaloruizdevilla/angular.js issue1949

Or view, comment on, or merge it at:

#1982
Commit Summary

  • fix(jqLite): IE8 no longer replaces link text content with href
    mailto value

File Changes

Patch Links:

@gonzaloruizdevilla
Copy link
Contributor Author

I'm going to try that approach

Also, I've found out that the bug is not produced because of the "mailto:", but instead is reproduced when using a text like an email. "a@a" also triggers the change in the text. I have to change PR code to take this into account.

…o value

fixes angular#1949
IE bug: http://webbugtrack.blogspot.com.es/2008/07/bug-140-changing-mailto-links-error-in.html
Only happens when:
1. content has an @ but not ends with it. Regexp: /.*\S.*@.+/
2. href new value is like a URI or email (more or less) Regexp: /^((ftp|https?):\/\/|mailto:|.*@.+)/

To prevent IE from replacing the text content, the text nodes must be detached
before setting the href value and reattached afterwards.

Signed-off-by: Gonzalo Ruiz de Villa <gonzaloruizdevilla@gmail.com>
@gonzaloruizdevilla
Copy link
Contributor Author

It turned out that the reflow trigger was not needed for the test, only a better understanding of the mysterious ways of IE8.

The fact is that IE8 replaces the content, if at least the following conditions exist:

  • content has an at sign and does not end with it
  • the value of "href" is like an URI or email (more or less)

This is why the code I posted in the comment didn't force the change, the link's text didn't contain an at sign.

Also, when jQuery is loaded in the page, this fix doesn't work. I will send it to jQuery too, they have a closed issue with this problem in "patchwelcome" status: http://bugs.jquery.com/ticket/8805

@IgorMinar
Copy link
Contributor

thanks for the pull request. Misko and I reviewed it and found an alternative solution that is lighter and fixes the bug for jquery as well. See #2013.

Your debugging however was super helpful! Thanks :-)

@IgorMinar IgorMinar closed this Feb 14, 2013
@gonzaloruizdevilla gonzaloruizdevilla deleted the issue1949 branch February 15, 2013 07:31
@gonzaloruizdevilla
Copy link
Contributor Author

You always find much better ways to solve the bugs than the ones I send in my PRs. :)
I greatly appreciate this, in my daily work nobody reviews my code, and it's very good for me to have this feedback and to learn better ways of doing things. Thanks to you :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IE8 mailto shows up in anchor text
3 participants