I agree. I will submit the patch. Thank for your specification on the overhead matter, haven't really thought of that (but as you said, performance was not a concern in this function, and I would assume that there won't be 1000's elements in the header object !)
Cheers. -Yanick On 14 fév, 14:53, Colin Mollenhour <[EMAIL PROTECTED]> wrote: > While I personally wouldn't use any library or code that broke "for... in" > loops, this is the real world and I think making the change you suggested is > a good idea. It would certainly help reduce the number of people coming to > the discussion board over and over for this same reason even though it > technically isn't Prototype's fault. Since this is not a performance-critical > piece of code, I see no reason not to implement your suggestion. Submit a > patch as described here (http://www.prototypejs.org/contribute) and provide > your best explanation for it in the patch description and hope everyone will > agree... :) > I'll be honest and point out that the Event patch I submitted uses for...in, > but I think it is justified since Event is a *very* performance critical > class and the overhead of $H and .each may not be worth it. I haven't tested > it, but I imagine a 1000+ element loop being called 1000+ times would make a > difference, in this case it won't make a difference. > Colin > Yanick wrote:This is what I did to solve the problem setRequestHeaders: > function() { .... // modified foreach iteration $H(headers).each( > function(header) { this.transport.setRequestHeader(header[0], header[1]); > }.bind(this) ); // for (var name in headers) // > this.transport.setRequestHeader(name, headers[name]); } The problem > encountered was that the foreach was trying to add the function 'extend' to > the headers, and was causing it to fail with an exception in the > XMLHttpRequest object under FF : "Component returned failure code: 0x80080057 > (NS_ERROR_ILLEGAL_VALUE) [nsIXMLHttpRequest.setRequestHeader]" (blah blah > blah) location: "JS frame ::http://.../prototype.js:: anonymous :: line 921" > data: no" I didn't do much extensive tests, but with the .each() function > hooked to a hash map of the header object seems to work nicely. ....And since > there's a .each() function, why not use it ? Just a thought. -Yanick On 9 > fév, 15:20,"[EMAIL PROTECTED]"<[EMAIL PROTECTED]>wrote:For the benefit of > anyone else with this issue... I ran into this when using rico 1.1.2 with > prototype 1.5.0. Same issue. I dropped rico off the page and > Ajax.Requeststarted working again. On Feb 6, 6:58 am, "Jimbo"<[EMAIL > PROTECTED]>wrote:Hi Christophe,100% guilty as charged! ;-) ... we're using > both json.js & prototype.js and I guess I never thought to look elsewhere > because it's something that changed when we moved from 1.4.0 to 1.5.0.This > seems a bit of a fundamental problem with json.js doesn't it?I see what you > mean though - and it extends Array and String in this way too ... is there a > good json serializer alternative that you know of or should I wait for the pt > implementationThanks anyhow,ATB, JimOn Feb 6, 11:34 am, Christophe > Porteneuve<[EMAIL PROTECTED]>wrote:Hey Jimbo,This is not a Prototype issue. > Besides Prototype, you're using a JSON-related library that patches > Object.prototype to provide it with a toJSONString method, > probablyhttp://www.json.org/json.js. This issue is widely held against its > official implementation...This breaks just about every for...in loop in every > piece of JS code you'll ever run when this lib is loaded (including the one > in setRequestHeaders). Which is why extending Object.prototype is widely > regarded as a malpractice. Actually, earlier versions of Prototype used to do > this, and quickly reverted to a cleaner behavior.Note that Prototype's trunk > (current development version) finally adds JSON-related methods, so in the > next point release you'll have them without the hassle. However, we use a > namespaced Object.toJSONString method for generic objects (and a regular > method for specific object types), to avoid this problem.-- Christophe > Porteneuve aka [EMAIL PROTECTED] --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Spinoffs" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/rubyonrails-spinoffs?hl=en -~----------~----~----~----~------~----~------~--~---
