[infusion] FLUID-3953: Revert throughout the framework to the use of $(el).prop("id" (#57)

[Justin Obara]

Hi Antranig,

I've merged your changes into the project repo at https://github.com/fluid-project/infusion/commit/ca843c2cffac362c095ffdf00f8e6529336b9a6d 

Do you know if jQuery and IE9 have been made aware, as you recommended?

Thanks
Justin

On 2011-05-31, at 2:02 AM, Antranig Basman wrote:

> On 30/05/2011 12:34, jobara wrote:
>> In the jquery 1.6.1 change log ( http://blog.jquery.com/2011/05/12/jquery-1-6-1-released/ ) they have a chart that shows the recommened uses of attr and prop. It seems like they suggest that you use attr for "id". Things seem to be working so it might be fine, but I suppose we should be careful about this.
>> 
>> Also it seems like not all of the uses of .attr("id") were switched to .prop.. not sure if this was intentional or if the were just missed. Here's the list of files where I found uses of attr ( Reorderer.js, InlineEditTests.js, Scheduler.js, SchedulerTests.js, TableOfContentsTests.js, FileQueueViewTests.js, SWFUploadManagerTests.js ). Also the same for getAttribute, ( portal.js, GeometricManagerTest.js )
>> 
>> I think we should have a proper unit test for  this in the framework unit tests..
>> 
>> I've tested this in IE6, IE7, IE8, safari and FF4 and it seems to have worked.
>> 
> 
> Thanks for this review, O KINGGG, and for catching these remaining uses in your characteristically thorough manner. The remaining use in Reorderer.js cannot safely be removed since this is a place where, for performance reasons, we are doing raw DOM iteration and trying to REMOVE the "attribute" for which there is not any particularly safe portable idiom (jQuery themselves say that in this case they try to "set the attribute value to undefined and catch any errors" which doesn't sound like something we want to experiment with right now). I have added the core unit test you suggested.
> 
> We should make sure that both the jQuery and IE9 communities are made aware of this issue - I would imagine that jQuery would change their recommendation for attr("id") based on this report - although see comment below - they may simply consider this issue as their bug.
> 
> Further experimentation showed that the call
> 
> element.getAttributeNS("", "id")
> 
> is also an acceptable replacement in this case for element.id on IE9 - if jQuery considered the IE9 behaviour a bug and also insists that we should continue with the use of element.attr("id"), perhaps this would be an acceptable replacement implementation for them in their framework code for jQuery.attr(). There may be portability issues on older browsers however.