[Prototype-core] Event.stopObserving, does it do proper cleanup?
I noticed that Event.stopObserving does not actually remove the array of [element,name,observer] from the observers cache that it adds when you call Event.observe. Could this be preventing proper garbage collection? Say you create a div and observe it with Event.observe and a function that contains a closure (e.g. bind()), then later remove that element from the DOM and all references to it that you have control over, there is still a reference to it in the observers cache and a reference to that function in the observers cache. Does this not prevent that memory from being regained after the stopObserving call completes? I don't think I understand javascript garbage collection completely.. This article is what got me thinking about this, I'm sure most of you have read it: http://javascript.crockford.com/memory/leak.html Thanks, Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Improved mass-observer handling
I have found that when creating an advanced prototype based class/widget, that all of the Event.observe and Event.stopObserving handling gets to be a pain in the ass.. I think it would be great to extend the current Event methods in Prototype to support some sort of "grouping" of events. Depending on the options, some do not always need to be removed so dispose functions typically look like: Event.stopObserving(...,...,...); Event.stopObserving(...,...,...); Event.stopObserving(...,...,...); if(this.blah) Event.stopObserving(...,...,...); if(this.blah) Event.stopObserving(...,...,...); I think a better scheme *in these cases* would be something like this when adding observers: Event.addObserver(key, element, name, observer, useCapture); and then, the dispose function would simply call something like Event.stopObservers(key); I'm thinking "this" would be a good key in most such cases. Does anyone see a problem with that? An added benefit here is that when extending classes that use this scheme, the programmer wouldn't have to worry about stopping his own observers as long as he used the same key. IPE comes to mind, all of the times I have extended createElement() functionality, adding observers and not having a good way to make sure they are disposed of later on unless I also extend IPE's own dispose(). This would also allow you to not have to store references to the event handler functions if you know you won't be manually removing it later: Event.addObserver(this,element,'click',this.clickElement.bindAsEventListener(this)); The handler function would be stored in the observers cache, and it could be unloaded as part of the stopObservers call. I'm just brainstorming at this point and will probably try to cook up some code and submit a patch, but I'd like to get some feedback. I think this would be a huge benefit to programmers like me who are making pages that have a potentially high amount of dynamic adding and removing going on. Thanks, Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Improved mass-observer handling
Ok, I've written the code to make this happen, it works great, is a pleasure to code, and has been submitted for your own viewing pleasure! I could use some help on test cases. I ran the unit tests but none of them are specific to Event.observe/Event.stopObserving and I need to write new ones for the new functions. http://dev.rubyonrails.org/ticket/7407 Thanks, Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Event.stopObserving, does it do proper cleanup?
Regardless of whether or not it causes a circular reference in IE, it does still keep some data in the cache which is a waste (considering things like tooltips are creating and destroying possibly many events in one second). So, I included this fix in my patch http://dev.rubyonrails.org/ticket/7407 I used splice() to remove the entire arguments array from the cache. Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Event.stopObserving, does it do proper cleanup?
I don't know if it is javascript, firebug, firefox or what, but after
running firefox for a while it's memory usage will creep up to over
250mb ("Mem Usage" and "VM Size" in the task manager). I have to
wonder if my Ajax heavy pages contribute to this so I'm not so
convinced that garbage collection is being done properly. Either that
or FF and/or the several extensions I have installed have some serious
memory leaks. Either way I haven't had the time or know-how to get to
the bottom of it but thought this may be a contributing factor. Every
time you Event.observe you are pushing an array containing an element
and a function into the cache array, and currently, prototype just
sets the element to null but not the handler function or the array
itself.
Colin
On Jan 27, 7:47 am, "Mislav Marohnić" <[EMAIL PROTECTED]>
wrote:
> On 1/27/07, Mislav Marohnić <[EMAIL PROTECTED]> wrote:
>
>
>
> > *Could* be preventing GC, ...Errr I meant "could be preventing proper GC
> > before unload", because on
> unload everything seems properly GC-ed anyway.
>
> -m
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] Re: Improved mass-observer handling
> First of all, I disapprove of marking #6509 as fixed because it isn't, the > patch is just superseded by another. You should have marked it as duplicate. Roget that. My bad, I'm still new to the Trac system.. > Second, I don't like breaking backwards compatibility with this additional > first argument. The method calls are starting to look too complicated. It doesn't break backwards compatibility, the original Event.observe and Event.stopObserving are still there, they call they new functions with the default key, Event.key. > With the Event rewrite by Andrew and me you only need to know what elements > you > added observers to and you can clean up like this: > > elements.invoke(Event.stopObserving) > > While your "key" idea is not bad, I don't think we should go to those > lengths just for better event handling. While this is a great idea and I'd like to see it included, it doesn't provide functionality for what I was trying to achieve. I want to be able to clean up all events for a widget or whatever else without having to know what elements have observers, or even having to know what elements have been added or are related to what I'm trying to cleanup. With my approach, widgets, classes, etc.. can be extended without any more complexity added to cleanup. I've already updated lots of my code to use this new system and it *really* cleans things up a lot. For simple things, it lets you not have to use this.someElement = ... and this.someListener = ... since all you have to know is the key. Now I overlooked this before, but "this" apparently isn't a good key as it seems that when used as a hash key it just evaluates to [object Object]. This doesn't change the usefulness of the new Event API, you just have to use something else for the key, such as the id of the main widget element. Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Improved mass-observer handling
> > I guess my point is: Keep the normal Event class simple and add extra
> > functionality in another layer.Very good point, Tobias. I like what you're
> > suggesting.
Thanks for the input, I've rewritten it with what, and I think you'll
agree, is a much better API.
New ticket: http://dev.rubyonrails.org/ticket/7435
I'll close the old one as soon as the Trac system starts responding
again...
This one uses an EventCache class to attain the grouping feature:
this.events = new EventCache();
It makes Event work just like any other instance of EventCache, so the
observe and stopObserving methods are now actually part of the
EventCache class, making the API both backwards compatible and
extremely familiar:
this.events.observe(element,'click',function(){}); // or the original
way:
Event.observe(element,'click',function(){});
I've also added the "wildcard" feature to stopObserving so you can do
any of the following in addition to the original API:
this.events.stopObserving(); //clears all observers in this particular
cache
Event.stopObserving(element); //clears all observers in the main cache
on this element
Event.stopObserving(element,'click'); //clears all 'click' observers
on this element in this cache
I think the cache separation is a major bonus to the new wildcard
feature as well. Without it, the wildcards could be VERY dangerous.
Imagine you have a tooltip class that is applied to a group of
elements and there is also say a hover effect applied to the same
group of elements via a different widget that has no knowledge of the
tooltip widgets. Removing listeners on one with a wildcard could
easily kill your other listeners *unintentionally* by calling:
Event.stopObserving(this.element);
but with the cache separation you could safely:
this.events.stopObserving();
without touching the other observers created by a different widget on
the same element!
Never has event handling been so easy and clean to my knowledge. I
looked at Mootools, jQuery, etc.. and none of them do anything to
protect against this kind of problem, which I can see happening *very*
easily.
Colin
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] Re: DOM builder in Prototype core?
> What do you think? Do you need it? Does it belong to core? If so, what
> approach should we take?
I personally use Builder.node quite a bit and find it very
convenient. I like the lightweight-ness of your solution but by the
time you add in the special cases it will have grown quite a bit I
think.
$E({
div: {
table: { attributes: {id: style: className: },
tr: {
}
}
}
})
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] Re: DOM builder in Prototype core?
I accidentally sent that last message.. was playing around with different API possibilities and hit tab and space and sent it... oops. Anyway, yeah, I like the idea but would like to see something that is syntactically prettier. I don't know what that would look like exactly, but perhaps some data structure to replace the nested function calls? Could you explain your "calls functions" feature? Thanks, Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Proposal: Custom Events / Messaging
Christian,
I've never programmed with such "global" events, but I like the idea
very much and can think of several places it could be useful in my own
projects. However, I don't think adding this functionality on top of
Prototype's Event is a good idea. Just like Tobias suggested to me on
my Event patch, add the new functionality in it's own class since it
really is different functionality altogether. This will avoid
confusion with the existing Event.observe API and keep the code
cleaner. I'd suggest something like "Message" or (I just looked up
dojo's API) "Topic".
Example API:
//in the vein of Ajax.Responders, this should already be familiar to
Protoype users
Topic.Responders.register({
onStart: loader.onCallbackStart,
onEnd: loader.onCallbackEnd
});
// subscribe to an event topic
Topic.subscribe('testEvent', function(){
alert('testEvent called with '+arguments.length+' arguments');
});
// publish the event to all subscribers
Topic.publish('testEvent', {bar:'foo'});
Of course you can use whatever names for the class and methods you
like, my point is just that this should be a separate API altogether.
This will let you avoid passing null parameters, etc..
Also, I suggest that registering an event name before using it *not*
be required, there is simply no need to.
What do you think of my suggestions?
Colin
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] Re: Proposal: Custom Events / Messaging
I just thought of a possible problem with the API I proposed.. As anyone who joined a large mailing list before knows, a great number of people cannot properly spell "unsubscribe". They would be stumped as to why their "Topic.unsuscribe" calls are causing errors... --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Proposal: Custom Events / Messaging
Dude, you should see a doctor.. On Feb 8, 9:06 am, "Ryan Gahl" <[EMAIL PROTECTED]> wrote: > *cough* *cough*, EventPublisher... *cough* *cough* links provided in this > thread... *cough* *cough* provides everything you're looking for *cough* > *cough* lightweight > > *cough* --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Another magical IE bug, breaks Element.extend
Good work! IE is one crazy mofo... --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: More Event Handling Propositions
> Colin, thanks for pointing out that my observer array will slow things > down in many cases. I really like your implementation, especially the > way you keep a different EventCache object. One thing I wonder, though, > if like you said your function returns an index--an array index could be > problematic, but why not use an object as a cache? Consider the draft > below. I like it! I like it so much, I blended all of your features with my own! See the patch here (updated): http://dev.rubyonrails.org/ticket/7435 I added the "return value is a cache key" feature, added your fixes to make UA checking happen only once (very nice!), and made Event and the caching model completely separate. All of this with complete backwards compatibility. I didn't include your "stopObservingElementEvent" and "stopObservingElement" functions, instead their functionality is included in stopObserving. Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: More Event Handling Propositions
I should point out that made the return value a simple int rather than
an object with a stopObserving function.
It would be quite easy to add, simply change (in EventCache.observe):
return this.cacheIndex;
to:
return {stopObserving: EventCache.stopObserving.bind(this,
this.cacheIndex)};
But, I don't think the overhead of a bind call and the storage of an
object with a function for EACH event observed is justifiable, just to
have the convenience of:
handle.stopObserving();
rather than:
Event.stopObserving(handle);
I seriously deal with cases that use thousands of event observers,
sometimes created in a short period of time, so this API needs to be
as efficient as possible.
As an aside, I just realized that this reply box in Google Groups has
a textarea that is resizable by dragging! Cool..
Colin
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] Re: More Event Handling Propositions
> I've been doing a lot of research on the subject of Event registration. > It made me question: do we really need to detach listeners in IE on > unload? Unless anyone knows for sure, I plan to test. According to what I've read primarily here: http://javascript.crockford.com/memory/leak.html it is pretty well established that it is necessary. Also, I think we can remove the try-catch blocks > on lines 116 and 125 of your patch because exceptions should never be > generated I put them in there because I thought IE's methods (or at least detachEvent) might throw an exception, but I honestly don't know for sure. Also, if an old browser doesn't support add/removeEventListener or attach/detachEvent then trying to call those functions would throw an exception, but that is a very minor point. About the Event.element patch, I've never used Safari as I don't have access to a Mac so I wouldn't know, but if you've read my recent posts you know how I feel about Safari... ;) It does look like it would make sense to do that if that is the case. I think this patch has become quite refined and mature by now, I'd love to get some more feedback from the Core guys... Thanks, Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: More Event Handling Propositions
>
> First of all, for all you joining late to the event discussion from
> several threads back; there's an Event patch [1] sitting in Trac for some
> time now that handles a lot. This and Colin's patch [2] don't overlap in
> functionality as much as in code; some clever merging will be required to
> take the best out of both patches. We had internal discussion about creating
> an event SVN branch in which these changes would be made - it may happen
> soon, depending on the people in charge of the repository.
Currently, your patch doesn't feel exactly "Prototypish".
First of all, I don't like splitting what we have into 2 things -- a
> namespace and a class -- when one can reside in another (eg. an event
> related class should reside in the Event namespace).
I broke it after Tobias' suggestion on the original thread, but it could be
merged back into the Event namespace easily enough using something like
Event.Group. Personally, I like the idea of the cache being a separate class
since the Event class seems to deal almost entirely with making things
cross-browser compatible and caching is somewhat of an add-on.
> Second, I don't like breaking chaining of observe().
I does *not* break Element.Methods.observe. I think this is the chaining you
are concerned about and it is completely intact. The chaining my latest
patch *does* break is just the chaining that I added in a previous patch
where each function returned a reference to the EventCache object so you
could chain multiple observes with different arguments. No functionality
from the original prototype is being lost here. However, there are some new
capabilities that can't be utilized when using the
Element.Methods.observechaining, specifically the return value as a
cache key, and the ability to
add the observer to a different cache. The latter could be remedied by
adding a new function like
element.addObserver(this.cache,'click',function(){}).
This isn't a big deal to me but it's your call.
Third, it has too much "private" properties (the underscore ones) and I
> can't stop thinking this featureset should take way *less* code.
All of the private properties are there for good reasons, either for
performance or to avoid redundancy. The only one that could be removed
without loss of functionality is the _observe one, but I put this in there
to achieve the separation of event handling and caching. I.e. if you wanted
to create your own caching model or not use caching at all you could use the
_observe and _stopObserving methods. Maybe for that reason these should be
private, but instead be named after the W3C standard? Event.addEventListenerand
Event.removeEventListener?
This doesn't mean I want you to "fix" your patch. No, it should stay the way
> it is now, but the ticket could stand tests. No, not unit tests (we're still
> figuring out how to unit-test events), but *functional* tests (the next best
> thing). Let the HTML document in [1] serve as the base for your tests. Keep
> em simple; there doesn't have to be much, just showing off the functionality
> and that it really works.
I'll get to running and writing tests as soon as I can (which may not be
very soon). In my unofficial tests (real world usage), they've been working
wonderfully. I could actually "feel" a speed boost in my app that builds and
destroys large trees of complex nodes, particularly when destroying. If the
patch were accepted and the S.A.U widgets were patched to take advantage of
the new cache model, the performance improvement would be even greater. It
mainly comes from not having to do a *linear search* on a huge array for the
cached records on stopObserving calls.
I'll read through your patch code again and see what can be merged between
the two as soon as the Trac system lets me download files... I have been
getting errors like crazy.
Thanks,
Colin
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] What is the reason for this?
While browsing through the prototype source I saw: 1686 var Toggle = new Object(); 1687 Toggle.display = Element.toggle; What is the point of this? I for one already have my own class called Toggle and I wouldn't be surprised if many other people do as well, but we are unknowingly overwriting a Prototype "class" that is undocumented and unnecessary. This isn't a huge deal until someone uses this in some useful library, and then it is broken. IMO there is nothing wrong with Element.toggle, so why steal another popular word from the global namespace? I would like to see this removed unless someone has a reason for this being there. If necessary, I'll even submit the patch but I think it should be removed before anyone uses it. Let's please leave aliases up to Prototype users and functionality up to Prototype Core. Thanks, Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: What is the reason for this?
Lol, nice.. I agree though, if someone doesn't care about Prototype enough to read the changelogs and blog, then why is he updating anyway? So does RoR use the Toggle.display method? Cuz I had never even seen it mentioned or used anywhere before. Colin On Feb 14, 6:51 am, "Mislav Marohnić" <[EMAIL PROTECTED]> wrote: > On 2/14/07, Richard Quadling <[EMAIL PROTECTED]> wrote: > > > > > Almost - but there are many, [dramatic pause] many fools out there. > > So we're talking about a person that keeps up to latest Prototype framework, > but doesn't read the blog, isn't interested in the changelog, ignores the > deprecation notices and is unaware of the compatibility addon? Where does he > live and why is he updating then? > > In short, my heart bleeds for that guy :P --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Proposed rewrite of $$/Selector
Man I just tried out your test page and WOW! I love the speed, that's what I'm talking about! I have refrained from using $$ before because speed is of utmost importance in my uses of Prototype, beyond that of elegance and maintainability unfortunately. I can't wait to have both! Mad props, Andrew. ;) Colin On Jan 19, 7:54 pm, "Andrew Dupont" <[EMAIL PROTECTED]> wrote: > I posted about this on Basecamp, but now that we've started to use this > list I'm going to post it here for public consumption. > > We've talked about optimizing $$ in the past -- it's one of my personal > goals for 1.5.1. So I took great interest in Jack Slocum's new > DomQuery extension for YUI > (http://www.jackslocum.com/blog/2007/01/11/domquery-css-selector-basic...). > Jack is a brilliant JavaScripter and has managed to write a really, > really fast CSS selector engine here. > > I took a look at his code -- it's quite clever, but also verbose and > inelegant in places. He handles a lot of specific CSS token > combinations by hand, which results in really fast querying but also > *lots* of lines of code. I resolved to write a version that was more > Prototypish. > > In the middle of doing so, I thought back to my earlier attempt with > XPath > (http://www.andrewdupont.net/2006/07/10/more-than-you-ever-wanted-to-k...), > and realized that I could add on an XPath approach with just a bit more > code. So I did. > > You can view the code at > (http://andrewdupont.net/test/double-dollar/selector.new.js). I've set > up a test page (expertly made by the jQuery team) that compares the > speed of the current $$ and my experimental $$: > (http://andrewdupont.net/test/double-dollar/). > > I'm still trying to make it better, but as I see it this new $$ solves > several problems with the current $$: > > (1) The current $$ does not filter out duplicates. > You can see this on the test page: "div div" and "div div div" both > return far more results than they should because certain nodes are > added to the collection more than once. Calling "uniq" on the array > before it's returned is *far* too costly, so I used Jack's inspired > method here: it enumerates the collection sets a property on each node, > so that if the function finds a node with that property already it > knows it's seen it before. (It then sets that property to "undefined" > before it's done.) > > (2) The current $$ is not very modular or extensible. > With this new implementation, I can add new tokens very easily -- for > example, all the operators for attribute matching (=, $=, ^=, *=, |=, > ~=) -- because they live in a hash with the operator as the key and a > comparator as the value. Similarly, adding new selectors like child > (>) and adjacency (+), or even pseudoclasses (:nth-child(even)) could > be done by adding a regex, an XPath translation, and a > string-of-JS-code translation. > > (3) The current $$ is SLOW. > XPath clearly solves this problem (try the tests in Firefox and see for > yourself), but not for Safari (version <2.0) or MSIE (version > *anything*). So even the "slow lane" needs to be faster here. Jack > claims his implementation is the fastest on earth, and though I've > taken his general approach I have not yet realized his gains. Still, > on costly selectors the new approach is almost twice as fast as the > current $$ (even leaving out XPath), and that's with more functionality > and fewer bugs (i.e., duplicated nodes). > > I'd love to hear some feedback. I've been looking at this code for way > too long now, so a fresh pair of eyes may point out something obvious > that I've missed. Also, I'd love it if someone were to modify this > code to add new stuff so that $$ can accommodate a wider range of > selectors. > > Cheers, > Andrew --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Ajax.Request Breaks on Firefox 2.0.0.1
Man this is getting annoying.. I wish json.org would change their code or Prototype would use $H().each in setRequestHeaders or both (preferable) would happen soon. This is popping up on Rails Spinoffs at least twice a day it seems. > You included the new JSON library from json.org? That is what broke > Prototype. --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: new Event functionality
I'm working on extending your patch with my EventCache feature. Your Observer class has grown on me so I'd prefer to just extend your code. :) I've been doing a lot of testing and one thing I've noticed is in IE, if you call Event.stopObserving with a function that isn't currently being observed but was at one time, you get an error: Error: 'this.element._observers[...][...].wrapper' is null or not an object Of course, you shouldn't be calling stopObserving in that manner, but it does happen and it shouldn't throw an error, so that line could either be wrapped in a try/catch, or some more error checking could be thrown in... Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: new Event functionality
> But it doesn't stop there. Ken Snyder and Colin Mollenhour have been
> collaborating on #7435 that enhances the Event module with a new cache
> system and some API extensions for bulk observer removing. Maybe some of
> those ideas are worthy, so I'd like to have other's opinions.
Ok, like I said earlier I liked your new Observer class and the many
improvements you added in 6195, but I can't stress enough how
incredibly convenient my cache system can be. Anyway, it's all for
naught if it doesn't run well, so I created a page for testing the
various event APIs that we've been discussing here:
http://localhost/colin/events/test.html?rev=hybrid
You'll note that the 1.5.0 is considerably faster, but the fact of the
matter is, it doesn't clean up properly. If you set it to 1
iterations and run the test a few times, you will see your browser's
memory usage soar to the sky. Both 6195 and 7435 fixed this but at a
performance cost. 7435 is actually *very( slow when not using the new
cache system to stop observers so I don't see it as a viable option
anymore. However, the new hybrid performs about the same as 6195 but
still adds the excellent EventCache API of 7435 with a few
improvements.
Case in point: The tooltip library I wrote that is used on that test
page. While the tooltip itself only has 2-3 observers, if you extend
it, like I did with Tooltip.popup, cleaning up those extra observers
suddenly becomes a PITA. The new convention I concocted for cleanup
can be used in any class is as follows:
- inside your initialize:
- this.events = new EventCache();
- this.disposables = [this.events];
- add events with this.events.observe(...)
- create a dispose function:
- while(this.disposables.length){ this.disposables.pop().dispose(); }
- do any other cleanup necessary, (usually nothing!)
- when extending your class:
- if adding events, use this.events.observe(...)
- if adding other classes, push to the disposables array:
this.disposables.push(new Draggable(this.element));
- all of the above will automatically be cleaned up for you!
This kind of cleanup API would make extending things like IPE with new
listeners and classes *much* easier. Basically, as long as you
implement dispose methods in your classes, cleanup is a one-liner.
That's only half the savings, since it also makes storing references
your your listeners manually no longer necessary. Inline function
lovers, rejoice!
Example: This will turn my tooltip class into a popup/draggable window
class without adding memory leaks:
ToolTip.popup = function(content, event, options){
options = Object.extend({
...
}, options || {});
var tooltip = new ToolTip(document.body, content, options);
tooltip.show(event);
tooltip.disposables.push(new Draggable(tooltip.content));
var target = (options.clickToDispose ? tooltip.content :
document.body);
tooltip.close = function(){ //add "close" method and onClose hook
this.dispose();
if(this.options.onClose){ this.options.onClose(); }
}.bind(tooltip);
tooltip.events.observe(target,'click', function(evt){
var element = $(Event.element(evt));
if( this.options.clickToDispose ||
(element != this.content && !
element.descendantOf(this.content))
){ this.close(); }
}.bindAsEventListener(tooltip));
return tooltip;
}
-
Say I want to now create a popup form that submits an Ajax request,
that's easy!
-
Event.observe(someElement,'click',function(clickevent){
var popup = ToolTip.popup(Builder.node('div',[Builder.node('form',[
Builder.node('textarea',{name:'text'}),
Builder.node('input',{type:'submit',value:'Sumbit'}),
Builder.node('input',{type:'reset',value:'cancel'})
])]),clickevent);
popup.events.observe(form,'submit',function(event){
Event.stop(event);
var form = $(Event.element(event));
new Ajax.Request('page.php',{parameters:form.serialize(true)});
this.dispose();
}.bindAsEventListener(popup));
popup.events.observe(form,'reset',popup.close);
});
-
Does anyone else see the value in this besides me? These are just a
few examples, I'm using this all over the place and it is so easy.
I haven't worked it into a patch yet, but the source code is here:
http://localhost/colin/events/event-hybrid.js
Changelog off of the top of my head (in addition to 6194):
- Observer is no longer sandboxed, but it could be just as easily. I
personally like it
available in case someone wanted to use it directly or with their
own cache system.
- Fixed a few bugs with event handling in IE
- Added a very general Cache class for convenience. Features:
- extended by Enumerable
- gives methods: inse
[Prototype-core] Re: new Event functionality
Oops, my links are localhost! lol... Here is the test page: http://colin.mollenhour.com/events/test.html And the new hybrid code: http://colin.mollenhour.com/events/event-hybrid.js Just in case it wasn't clear before, "hybrid" is Mislav's and Andrew's 6194 patch that has been slightly reconfigured to use my EventCache API with all of 6194's juicy features including an approach to Ken Snyder's suggestion to make use of a return value. Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: new Event functionality
Well, I had written a nice long response to your critique and GC system and then FireFox decided to crash (I am very pissed) so this response will be a bit shorter and more blunt.. I don't see how your proposed GC system supercedes my EventCache proposal or makes event cleanup simpler. Perhaps you'd be willing to demonstrate a simple example of your system by rewriting my ToolTip.popup extension? (without making modifications to the original ToolTip.prototype that are specific to the extension) I will be glad to add it to my test page to see how well it performs and study the design paradigm used. http://colin.mollenhour.com/events/tooltip-lite-6194.js You say I am coupling GC and event handling too closely but what I am really doing is *de*coupling the two and offering a default solution to keep in tune with Prototype's current API. On top of that I am simply proposing a coding convention that can be easily used to make widgets (or whatever) dispose their children without a bunch of work from the programmer. The model is simple, if node A contains children B, C and D and node A is to be disposed, then it's children B, C and D should be disposed of as well. Likewise, if B contains E, F and G, then when A disposes B, B should dispose E, F and G without A having to care. What can be simpler and more intuitive than that. *If* I want to make any part of it "asynchronous" I can do so manually via a one- line setTimeout wrapper inside the dispose function as appropriate. Giving EventCache a dispose() method is optional, but it makes sense to me since a widget's events need to be manually disposed of just as if they were child widgets themselves. Ideally JS's own GC would handle this but alas, it does not so this is my best solution. You bring up scalability and complexity... My approach was born out of a real world situation that is more complex than even your fictional (perhaps not) example. I have used this with my Ajax.Tree class in live situations where there are at least six different types of tree nodes, each containing their own widgets such as IPE, tooltips, buttons, mouseovers, forms, etc.. The trees constructed of these nodes have easily over 100 nodes and the entire tree or subtrees may be refreshed (destroyed and recreated) as a result of the user building a new query or expanding/collapsing a parent node. Asynchronicity in JS has very limited use and is even inappropriate in cases such as this. Performance is quite reasonable but IE does suffer slightly. Regardless, the old nodes need to be cleaned up before the new ones appear and the only other option is to ignore the event cleanup altogether (innerHTML = '') and let your browser's memory usage skyrocket. <-(not a good solution) Asynchronous operations work great in environments that support true threading such as Java, but not as much in JS. E.g. with a dual core CPU you cannot cause CPU usage to surpass the 50% mark using JS code. Also, there is no preemption, so using setTimeout to gain "asynchronous" behavior is an illusion. Sure, it "frees up the UI", but to do what? Call two functions that each take 15 seconds to complete via setTimeout and all you have done is add a little non- determinism to the system since each function will execute from beginning to end without interruption from any other JS code. Thanks for the comments, Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: new Event functionality
Lightweight indeed! I've been looking for something like this, and I would have questioned before whether or not it belongs in Prototype but it is so short and sweet that it certainly wouldn't do any harm to add it. +1 for this solution, I really like the simplicity. Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: new Event functionality
That's just it, I don't fully understand how your GC class is supposed to work so I was hoping to see a real-world usage example, but if you are too busy I can understand that completely (maybe you have a page up already that you could provide a link to?). I think your focus is on scalability and performance, and while I do consider those things important, my focus is really more on making *event* cleanup easier by requiring fewer lines of code and providing a hassle-free way of queuing up event handlers to be disposed of correctly. I appreciate your help and suggestions but from what I understand your GC class has nothing to do with events specifically. I would probably have given up by now on getting this added to Prototype if it weren't for the tight coupling of events and caching in Prototype's current system. If these were at least separated then I could add EventCache on top of Prototype but they are not at all so that would currently require overriding pretty much the entire Event class. Just from a modular code design standpoint, can we all not agree that event handler methods and event handler caching are better off separated to a certain degree? I'm not suggesting a new API, just a new caching class that gives the user more control over caching when they want it. Instead of having one huge cache for every event, I am proposing the default cache (Event) and then a class for creating new caches to make cleanup by the user easier. Am I asking too much or am I just going crazy? Thanks, Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: new Event functionality
Thanks for taking the time to write that up. I think you and I are more on the same page than we thought, your example looks very similar in structure to my example. I would be completely happy with what you proposed except for the redundancy. Since Prototype already does global caching (only reason being *currently* is for IE cleanup) I don't think there is a good clean way to do your own caching without keeping duplicate references to everything which is something I really don't want to do. Mislav's and Andrew's patch has the same characteristics as the original Event class in that the global caching is still done internally (although only for IE). My patch separates the global caching from the adding/removing process (keeping the original API for backwards compatibility). Also, I should point out that the 6194 build has a pretty major problem in IE... When you call stopObserving, the Observer is never removed from the globalCache! This is still an improvement over the original cache system which doesn't remove the [element,type,handler,useCapture] array from the global cache in either IE or FF! My changes to the 6194 build fixes this and another IE related bug. Mislav, how do I submit a patch to the new event branch? I'd really like you to look at the changes I made in http://colin.mollenhour.com/events/event-hybrid.js but that would probably be a lot easier if I submitted a formal patch. Thanks, Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: new Event functionality
I'll submit a patch, I just wasn't sure if patches to branches were
submitted differently than patches to the trunk.. and I'll start using
Pastie :)
The gist of my patch is this:
-adds an EventCache class that provides the observe/stopObserving
methods
-the EventCache methods handle creating/removing Observer instances
and storing these instances of Observer in a local cache
-Event is extended by EventCache so Event.observe/stopObserving work
as before
-A method to dispose of all EventCache instances will do global
cleanup on IE
-A few other changes not mentioned here for brevity
The net effect is that you can create multiple EventCache instances
and dispose of all event handlers in a particular cache with one call:
var events = new EventCache();
events.observe(element,'click',funciton(){});
etc...
events.dispose(); //cleans up all events added to this cache
Benefits:
-The dispose method has less overhead than separate stopObserving
calls
-Give each widget it's own cache and the following code:
this.someListener = this.someFunction.bind(this);
Event.observe(this.someElement,'click',this.someListener);
Event.stopObserving(this.someElement,'click',this.someListener);
can be replaced with:
this.events.observe(this.someElement,'click',this.someFunction.bind(this));
//no separate stopObserving call is necessary
-Now, adding other events to a widget that has it's own cache is
*much* easier:
widget.events.observe(element,'click',function(){});
The above observer will be disposed of properly when the widget is
disposed of.
Note, this functionality cannot be replicated with the 6194 code by
calling Event.stopObserving(this.element). Here's why:
-There are likely more than one elements that have observers needing
disposal by a widget
-The element may have observers that shouldn't be disposed of (created
by other widgets on the same element for example)
Lastly, why don't I add this functionality separately instead of
trying to get Prototype patched? It is because the Event class does
it's own caching in such a way that cannot be extended, so such
efforts would be duplicating the caching and over-complicating cleanup
procedures.
Let me know if you have any other questions or thoughts on this. I
will try to get a patch submitted on Trac shortly, but to see what the
new event.js file would pretty much look like you can go here:
http://colin.mollenhour.com/events/event-hybrid.js
Thanks for the consideration,
Colin
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] Re: new Event functionality
> Maybe we can expose the (currently hidden) Observer class, allowing for a > plugin? I couldn't complain about that now, could I? :) I still think it would be great to add to core, because then SAU and other widgets could be patched to take advantage of the separate caching benefits, but I can't seem to get anyone too excited about that... If I were to submit a patch that exposed the Observer class and separated it from the global caching but otherwise basically kept everything the same would you be more likely to accept it? Although it's kinda funny because the *only* difference between that and the code in event-hybrid.js would be that there is one global cache as a property of Event rather than an EventCache class that can be instantiated multiple times, and it would be about 40-50 lines shorter. Thanks, Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: new Event functionality
> This is where I was taking issue before. You incur no real performance or > memory hits by keeping an extra reference (if you wanted a local caching > policy for a class, that is). 2 references to the same object do not more > memory take. There is no reason to maintain two separate caches where one will suffice. It is messier, and I really question your claim that there is no real performance hit. If you check out my test page, you will see that the 6194 code is considerably slower in adding and removing observers than the original prototype code (it is worth it though to do proper cleanup). Obviously, looking up, adding and removing references takes time, and doing this twice will take... twice as much time. > Good luck with the patch. Thanks. :) Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: new Event functionality
It's good to know I'm not *totally* out of my mind... :) Thanks, Colin > I'm not a core developer, so my opinion has no weight ... but I like > the concept. Really. =) > > I think it would cleanup the widget destroy functions. For example, > complex Sortables. > > TAG --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Hash.toQueryString changes
Is 2 not incorrect? That should be "foo[]=a&foo[]=b" if I'm not
mistaken... On the server side you will get foo=b. Similarly for 3 you
will get foo=c.
I haven't looked at the code or thought about it much, but I would
like to see recursion of hashes and arrays. If you do 4, I would guess
that taking it all the way shouldn't be much harder.
{ foo:{bar:['a','b','c']}} => "foo[bar][]=a&foo[bar][]=b&foo[bar][]=c"
As for arrays, I think they should be given indexes because it would
more accurately reproduce the original data structure and with a good
recursion scheme probably be easier to code (but that's just
speculation).
{ foo:[['a','b'],['c','d']]} => "foo[0][0]=a&foo[0][1]=b&foo[1]
[0]=c&foo[1][1]=d"
{ foo:['a', ['b'], 'c']} => "foo[0]=a&foo[1][0]=b&foo[2]=c"
{ foo:['a',{bar:'baz',baz:'zab'},'c']} => "foo[0]=a&foo[1]
[bar]=baz&foo[1][baz]=zab&foo[2]=c"
I would probably find this stuff very useful in the future if it was
fully featured.
I've setup a simple test page to test such query strings here:
http://colin.mollenhour.com/posttest.php
I assume pretty much every server-side language will treat these the
same way, and performance probably isn't critical, so why not go the
whole nine? Just my .02
Colin
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] Re: Hash.toQueryString changes
Yes, as I stated in my previous post as well, this *definitely* need
to be changed to follow the array name with brackets, otherwise you
are simply redefining that variable over and over again and it will
simply equal the last value. Maybe rails handles this correctly but
probably nothing else will and it simply isn't correct. I thought we
all knew that to make an array in a query string you use []?
A simple test of the 1.5.1_rc1 code shows that
$H({blah:['foo','bar']}).toQueryString() => "blah=foo&blah=bar"
If you paste this into my test page [1] you will see that php gets:
[blah] => bar
The correct result of the above toQueryString should be:
"blah[]=foo&blah[]=bar"
Which will yield:
[blah] => Array (
[0] => foo
[1] => bar
)
Did anyone read my first post to this thread? I make some very good
points there but they seem to have been completely ignored thus far.
[1] http://colin.mollenhour.com/posttest.php
Colin
On Mar 14, 8:06 am, "Richard Quadling" <[EMAIL PROTECTED]>
wrote:
> On 13/03/07, Mislav Marohnić <[EMAIL PROTECTED]> wrote:
>
> > "foo=a&foo=b&foo=c" becomes { foo:['a', 'b', 'c'] }. Rails (PHP too, I
> > think) would preserve only the first value. Ruby preserves all.
>
> In PHP, this would end up as
>
> array(1) { ["foo"]=> string(1) "c" }
>
> $_GET is an array of 1 element with an index of 'foo' with a value of 'c'.
>
> > "foo[]=a&foo[]=b&foo[]=c" becomes { 'foo[]':['a', 'b', 'c'] }. Rails and PHP
> > would now preserve all values, but remove the square brackets from the key.
>
> In PHP, this would end up as
>
> array(1) { ["foo"]=> array(3) { [0]=> string(1) "a" [1]=> string(1)
> "b" [2]=> string(1) "c" } }
>
> $_GET is an array of 1 element with an index of 'foo' which is an
> array of 3 elements, 'a', 'b' and 'c'.
>
> If you have PHP, use this as showget.php
>
>
>
> --
> -
> Richard Quadling
> Zend Certified Engineer :http://zend.com/zce.php?c=ZEND002498&r=213474731
> "Standing on the shoulders of some very clever giants!"
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] Re: Hash.toQueryString changes
Trac isn't responding at the moment so I can't explore and see what other problems Hash.toQueryString is having, but I completely rewrote it myself to support nested structures and the rewrite handles all of the cases mentioned in this thread correctly (including for servers out there that aren't running Rails). The string returned reproduces the original data structure on the server-side flawlessly and is only 26 lines of code total! For your consideration, I've included the code and a test page that allows you to run your own tests and includes a good default test case that also shows off it's abilities nicely. It can be tested alongside the Prototype 1.5.1_rc1 version which obviously does not encode arrays correctly and cannot encode the nested structures. Code: http://pastie.caboo.se/47059 Test: http://colin.mollenhour.com/posttest.php I'd appreciate your opinions and tests if you think you can break it. Thanks, Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Hash.toQueryString changes
WTF!?!?!? This is the fifth time I have posted a reply to a thread via google groups and it says the post was successful and then it never shows up!! Ok, I honestly don't care anymore as this is way more trouble than it is worth. It is much easier to maintain a file of all of my personal patches than to get anything changed in the core because I have to argue my point endlessly and even *if* I'm right it doesn't matter.. My first response was much more polite but I am in a really bad mood now because I just waisted a few hours setting up a rails server and a test page and writing responses, so I apologize for for my tone. Look, I installed and tested this on Rails (using CGIMethods.parse_query_parameters) just for this thread and it NEEDS brackets, ok? I went into details before but google groups said "No sir! haha!" so I won't go into it again. If you think you're right, run a test and prove it. I'd like to see tests on other platforms as well to see how brackets and numeric indexes are handled (PHP as arrays, Rails as hashes) but I'll leave that up to people on those platforms who give a damn (probably nobody). Michael, care to copy and paste my test page code into some tests for some of the other platforms you mentioned? Just dump the post variable if it exists, otherwise print out the page. Why not make this code work with as many server-side languages as possible and THEN work on the toQueryParams function? --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Hash.toQueryString changes
> Try this "echo" service:
>
> request:http://www.neohub.com/ws/echo?foo=1;foo=2;foo=3;bar=4
>
> answer:
> {'bar':4,'foo':[1,2,3]}
Would you care to mention what platform and what method of parsing
into variables is being used in that test? The results are different
than my Rails test.
> Hehe, now you reminded me that I wasted some time today too reading all the
> crap^Wstuff athttp://www.php.net/manual/en/language.variables.external.php
> trying to understand how does your PHP bracket convention really work.
> Look quite clear that if you want arrays, you have to apply some magic to
> HTML "name" attributes (adding "[]").
That's why I setup my PHP test page for everyone to use...
> OTOH, as I said in my previous message, folks here seem to be trying making
> up some abstract serialization engine, so this discussion is bogus, as you
> don't want a perfect Form.serialize(), but just to apply some PHPism into
> this application/x-www-form-urlencoded serialization engine. Why don't you
> keep on using the same convention you already use in your HTML with this
> toy? It should work the same. For instance:
>
> >>> $H({'foo[]':[1,2,3]}).toQueryString()
For one, that is hackish and not always possible to do. Also, to
support nesting of arrays you *must* use brackets with indexes so if
you just want reliable serialization and deserialization the brackets
with indexes are the most reliable method. How else can you do this
correctly?
{foo:[[0,1],[2,3]]}
Colin
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] Re: Hash.toQueryString changes
> Here's mine. First, it uses $H(value).findAll, so it isn't safe (try feeding
> it "each", for instance).
The new function in 1.5.1_rc1 has the same problem so I figured you
were comfortable with that fact already.. I don't see a good solution
to this as long as hash prototypes are extended.
> Second, about arrays:
>
> Hash._toQueryString(key+'['+index+']',val);
>
> I never understood why to put index in there. That complicates both
> serialization and parsing (think toQueryParams), plus we shouldn't be
> changing keys because it totally breaks form serialization.
That code will only get called when there is an actual JS array nested
inside a value. So, for HTML form serialization I don't see how this
code will ever be called anyway. It isn't changing the intended "name"
attributes in the form, if that is what you were alluding to. This
feature is really for when you build an array yourself in JS, not for
automatic form serialization.
> I like how it supports nested hashes, though. We still haven't agreed on
> this internally, but I think nested hashes support may be in version 1.6.
Sounds good, I can't think of a strong reason not to..
> I also think we need Array.prototype.toQueryString to move the logic there.
That doesn't make sense though... The encoding requires a key and
doing something like:
[5,6,7].toQueryString() doesn't specify a key anywhere. AFAIK there is
no correct way to serialize this.
The correct way for the programmer would be:
$H({mykey:[5,6,7]}).toQueryString()
Now the question seems to be to use brackets or not, and to use
indexes or not..
I am for using indexes (mykey[0]=5...). In Rails (in my tests) this
would give you a hash with indexes being "0", "1", "2". Is there any
reason that isn't doable? Treat the hash as if it was an array, order
is preserved, indexes are no different, probably more languages will
play well with this method than any other, everyone is happy, no?
Deserializing is at least well defined this way and shouldn't be too
hard, just test the keys to see if they're numeric and create either
an array or hash and fill it up.
I personally don't see a great need for deserializing.. If the
programmer is smart enough to keep everything as an object until the
very last step (sending it to the server) then there should be no
need. If you want to serialize and deserialize to watch for changes or
some other internal use, why not use the new JSON functions? JSON is a
much better encoding scheme than application/x-www-form-urlencoded.
For Ajax.Request, I think deserializing a passed string and then
reserializing is a bad idea. Why muck with the programmers data? If
they pass a string, just assume they know what they're doing and keep
it a string.
> Moreover, why not deprecating toQueryString/Params in favor of "toQuery"?
I'll let you guys make the API decisions, but the current API suits me
fine. :)
Thanks for your opinions and code review, Mislav.
Colin
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] Re: Hash.toQueryString changes
After seeing all of the server-side inconsistencies with url decoding, I too agree that Hash.toQueryString is getting too complicated. In fact, what was wrong with the old one? The one currently in 1.5.1_rc2 needs to go as it has several problems that the old one didn't have and it is pretty convoluted for what it accomplishes. It'd be nice to be able to encode any data structure but it doesn't look like it is going to work. Also, why can't Ajax.Request leave the parameters option a string if it is passed a string? Converting to a hash and back to a string is completely unnecessary and complicates things. If the user passes a string, let's just assume he knows what he's doing and doesn't want his string broken down and reconstructed. If an object is passed it can still be converted to a string. That way problems like using enumerable method names as keys can be circumvented by the user building the parameters string manually. Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Node Insertion Methods
Mislav, what are the general plans for the Insertion rewrite? Aren't
these totally different things? Insertion is for HTML snippets and
these are pure DOM methods.
Ken, a few suggestions for your code..
Don't use .down and .next with no arguments unless you need non-text
nodes. It is very inefficient in this case. next grabs all nextSibling
elements recursively (potentially many, extending each one in the
process) and then just returns the first one. down grabs *all*
descendants, then loops through extending each one, and then returns
the first one.
I suggest only using those methods if you need guarantees that you are
getting a DOM element and not a text node. In this case I would argue
that you *do* want text nodes. Consider:
This is the content text
This is a paragraph
var newNode = document.createElement('div');
newNode.innerHTML = 'This is the new content';
$('content).prependChild(newNode);
//would result in:
This is the content text
This is the new content
This is a paragraph
That is probably not the desired effect, at least since you are trying
to follow the behavior of appendChild and insertBefore. Go back to
nextSibling and firstChild for these functions.
This is moot if you consider my first suggestion, but where you used
refNode.next() you don't ensure that refNode is extended so you'll get
"no such property" errors on elements that haven't explicitly been
extended by Prototype yet.
Lastly, if this is to become Prototype code, it should follow the
Prototype conventions and accept strings representing ids for any
argument that is to be an element, and they should be chainable
anywhere that it makes sense so really we just need a fresh new API
for this stuff. I like the mootools implementation, simple and
functional.
Colin
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] CSS syntax for node creation?
Just a thought, if a future version of prototype were to include a
simple element creation method, what about using CSS syntax for
specification of id and class? i.e.:
var node = new Element('div#mynode.someClass');
Could the new CSS3 parser be easily reused to implement this? I looked
at it briefly but it's pretty complex :)
Good idea or bad?
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] Re: Hash.toQueryString changes
If we are talking about form serialization, then when is it even
possible to have null or undefined values? Form.serialize grabs all
form elements, so if it does it's job correctly then undefined is out
of the picture. On FF (haven't tested else where or looked up specs)
the .value property cannot be null.. create a new element and don't
assign it a value and it will evaluate to "" already. Even when I
manually assigned it to null it still evaluates to "". Is there a case
in which a form element's value can be null? Before you say "the user
can define null and undefined values directly in a hash", read on.
Creating arrays in an HTML form is really just a hack, defining
elements with the same "name" value repeatedly.. As we've seen,
interpreting this is not a well-defined operation. For example, a PHP
programmer (or others) may intend for one element's value to override
another if present by reusing the name without brackets while a Rails
programmer would expect an array. It seems like forms are better
represented as a collection of independent key->value pairs rather
than a hash, which are not the same things.
1) A hash is not guaranteed to maintain the original order in which
the elements were defined, and
2) a hash is composed of unique keys that have one value.
Serialization and deserialization of a collection of such key->value
pairs would be completely straightforward, and I suggest this is the
way it should be done. This is the only truly server-agnostic solution
IMO. I would then suggest providing a separate method for interpreting
that collection either as a strict hash, or as a hash with nested
arrays in the case of multiple definitions of the same key.
Example:
//PHP users would most *likely* use strict = true
//default result is same as current serializeElements([],true)
functionality
collectionToHash: function(collection, strict){
return collection.inject({}, function(result, pair) {
if (!strict && pair.key in result) {
if (result[pair.key].constructor != Array) result[pair.key] =
[result[pair.key]];
result[pair.key].push(pair.value);
}else result[pair.key] = value;
return result;
});
}
Doing the above would then allow for much better flexibility in
toQueryString and toQueryParams.. i.e. "real" arrays would be given
indexes and then any combination of arrays and hashes would be
possible with the caveat that some server-side languages will
interpret numerical indexes as hash keys rather than array indexes but
as I already said, that is a shortcoming of the server-side
interpretation.
Of course, code could be shuffled around a bit so that backwards
compatibility is maintained.. for example, serialize could get another
optional parameter which would simply be passed on to
collectionToHash, but for internal use, direct serialization of a form
to a collection to a string wouldn't care.
serialize(form,getHash,strict)
Thoughts? Basically I am separating Hash.toQueryString from
Form.serialize because forms are not most accurately represented as
hashes to begin with, putting the question of server-side or intended
interpretation out of the picture and therefore allowing toQueryString
to do it's thing independently of form serialization.
I'd be happy to write up a patch that fully implements what I've
proposed if the core team is interested.
Thanks,
Colin
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] Re: Hash.toQueryString changes
I've written new code that I think will offer the most functionality
to the most people. The only functions modified are
Form.serializeElements, Form.Methods.serialize, Hash.toQueryString and
String.prototype.toQueryParams.
- Forms are serialized directly to strings so that there is no hash
middle-step to introduce inconsistencies favoring any server-side
technology
- When creating a hash from a string, an optional parameter
"strict" (use true) will produce:
foo=bar&foo=baz {foo:['baz']} rather than {foo:['bar','baz']} (the
default)
foo[]=bar&foo[]=baz is still {foo:['bar','baz']} in both cases
foo[0]=bar&foo[1]=baz is also supported to enable nesting
- Hash.toQueryString will add indexes to arrays which allows for
complete nesting of arrays and hashes. This shouldn't be a problem for
anyone when serializing forms because this is not used during form
serialization.
- String.prototype.toQueryParams can correctly deserialize any string
produced by toQueryString, even those with nested hashes and arrays
(only possible with explicit array indexes).
http://colin.mollenhour.com/toquery.php
http://pastie.caboo.se/48450
Does anyone have any problems with this proposal?
Thanks,
Colin
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] Form.serialize, Ajax.Request, Hash.toQueryString, "".toQueryParams
I started a new thread on this since it was getting quite long and I
now have a new take on the subject.
The changes to these functions introduced in 1.5.1 have broken a few
things, no doubt about that. The problem is Ajax.Request always
converts parameters into a Hash, making *assumptions* along the way
about how to serialize and deserialize it. It is very disconcerting to
pass in your parameters string and something else entirely gets passed
to the server. The only way one can guarantee string is not fouled up
is to change "parameters:" to "postBody:" everywhere, but I believe
this really shouldn't be necessary. At the bare minimum, the behavior
of "parameters" should be documented to warn that it's use with a
string could result in changes to your final query string but
preferably a string would remain a string throughout.
About these assumptions, I am talking about the difference between:
'foo=bar&foo=baz' => {foo:['bar','baz']}
'foo=bar&foo=baz' => {foo:'baz'}
and also
{foo:['bar','baz']} => 'foo=bar&foo=baz'
{foo:['bar','baz']} => 'foo[0]=bar&foo[1]=baz'
Which will work is dependent entirely upon your server-side platform.
For form encoding, the only thing we know is in the case of , the name is repeated and no brackets are added. So, the
default behavior should be to not add brackets, and certainly in the
case of form serialization, as it is important to mimic the browser's
own serialization as closely as possible.
So, Form.serialize(form) returns a string directly (without building a
hash first). This gets rid of the first problem when serializing forms
to a string.
Now, when Form.serialize returns a hash, I added an additional
parameter that will determine which method will be used in decoding
the string. I call the latter method "strict", because when in strict
mode, pre-existing keys are reassigned rather than converted to arrays
so the result is strictly a hash of keys->values. The default is
consistent with the original API and with Rails. If form names have
brackets appended manually it will still be compatible here as well.
Similarly, if you expect your duplicate keys to be converted into
arrays then you probably want your arrays to be decoded back into
duplicate keys. This poses a great limitation on the power of
toQueryString since nesting anything inside an array would be
impossible to encode and decode without adding indexes explicitly. For
example:
{options: [
{text: 'First option', value: 24},
{text: 'Second option', value: 56}
],
default: 24
}
This can only be properly encoded as
options[0][text]=First+option&options[0][value]=24&options[1]
[text]=Second+option&options[1][value]=56&default=24
My solution to this was to check the array in question, if it has
nested arrays or hashes, I add an index manually, otherwise I don't.
Since HTML forms will never have nested arrays this works perfectly
for everyone but still allows you to do nesting like in the example
above. I also added an optional parameter to toQueryString that will
always add brackets so that platforms that require them don't require
hacking up your object keys. The default again, is consistent with the
original API and Rails and for form serialization will produce the
correct result each time.
I've also removed the use of $H functions from toQueryString for
compatibility with keys such as "each", "collect", etc..
Test page for form serialization: http://colin.mollenhour.com/querytest.php
Test page for hash->string->hash conversion:
http://colin.mollenhour.com/toquery.php
Code: http://pastie.caboo.se/48953
Thoughts, opinions?
I suppose some of you won't like the extra parameters, but without
them it is really limiting the usability of these functions in many
cases.
Thanks,
Colin
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Prototype: Core" group.
To post to this group, send email to prototype-core@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/prototype-core?hl=en
-~--~~~~--~~--~--~---
[Prototype-core] Re: Form.serialize, Ajax.Request, Hash.toQueryString, "".toQueryParams
Sweet, no replies.. I guess I've worn out my welcome with the Prototype Core team. No problem, I'll just go back to waiting for final versions and keep my opinions and code to myself. Oh I've always wanted to say this, but Christophe your contributions to the Spinoffs list are amazing, I don't know how you find the time... Over and out, Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Form.serialize, Ajax.Request, Hash.toQueryString, "".toQueryParams
Re using JSON: if you're going to do this you might as well encode it all as JSON in the first place don't you think since that would require extra attention on the server-side anyway? Mixing encoding schemes, really? I was hoping a solution that was compatible with nearly all servers and was still robust would be possible, but the old code's features were adequate before so it really isn't that big an issue. Either make it support full nesting, or don't. Throwing JSON into the mix is worse than simply not supporting nested structures, IMHO. Of course this is assuming that form serialization is done as I proposed rather than via Hash.toQueryString. I guess I'll find out soon enough. Thanks for the comments, Colin --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Form.serialize, Ajax.Request, Hash.toQueryString, "".toQueryParams
Awesome, I'll check it out (literally). Thanks, Colin On Mar 28, 6:36 am, "Mislav Marohnić" <[EMAIL PROTECTED]> wrote: > Collin (and others) - > > Please check out the latest trunk. Yesterday's update fixes the final minor > issues:http://dev.rubyonrails.org/changeset/6481 > > See the unit tests for Hash#toQueryString and String#toQueryParams. Let me > know if there's something missing. > > We're not supporting nested hashes and arrays yet. If there is going to be > need to, we may introduce it in time for Prototype 1.6. --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
[Prototype-core] Re: Image File Serialize
See this for a start. It is nowhere near Prototype Core quality but it works. http://pastie.caboo.se/50410 Colin On Mar 28, 9:02 am, Stefan Liebenberg <[EMAIL PROTECTED]> wrote: > Hey > > Does anybody know how to send a serialized upload file value thing to > your server I tried and i suck. > > Stefan --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Prototype: Core" group. To post to this group, send email to prototype-core@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/prototype-core?hl=en -~--~~~~--~~--~--~---
