ArcBees / gwtquery

A jQuery clone for GWT, and much more.
MIT License
85 stars 38 forks source link

removeAttr() doesn't remove attribute #196

Closed jDramaix closed 11 years ago

jDramaix commented 11 years ago

Original author: julien.d...@gmail.com (December 17, 2012 23:26:16)

<a href="#" title="a title">anchor</a>

$("a").removeAttr("title");

We expect that the anchor doesn't have title attribute anymore. But the title is just set to an empty value. The side effect is that IE displays a native tooltip with the string null as content.

Original issue: http://code.google.com/p/gwtquery/issues/detail?id=165

jDramaix commented 11 years ago

From kisalay...@gmail.com on January 16, 2013 13:33:42 This seems to be a problem with GWT itself. It works fine in Chrome, but in Firefox, i tried calling GWT's Element object's removeAttribute because that is what is finally called. The results were the same.

Anyways, the gwtQuery's code is removing attribute first and then checking on whether it is a boolean or not(as done in JQuery code). Seems a bit redundent. Changed that (as it is done in Jquery)...(see patch)

jDramaix commented 11 years ago

From julien.d...@gmail.com on January 16, 2013 13:57:24 Hi kisalaya89,

Thanks for the patch. Can I kindly ask you to submit a pull request on the project on github ? : https://github.com/gwtquery/gwtquery

That make the code review and merge mechanism easier.

jDramaix commented 11 years ago

From manuel.carrasco.m on February 07, 2013 14:48:14 Although the patch fixes this issue, it breaks other cases like gq.removeAttr("class") which is in the test suite.

I think the correct fix is moving the e.removeAttribute(key) from the line # 173 to # 181

jDramaix commented 11 years ago

From kisalay...@gmail.com on February 07, 2013 15:16:23 Hi manuel.carrasco.m,

This is the updated patch. It is according to what you said in above comment, and it works.

jDramaix commented 11 years ago

From manuel.carrasco.m on February 11, 2013 13:52:56 This issue was closed by revision 6c36a6ef2221.