Changes to Form in r5449
jonatha...@gmail.com
weren't actually forms. I think this was actually a good thing, you
could do:
Form.disable('my_area')
This would disable all form elements in my_area, even though my_area
was not a form. As of r5449, Form.disable (as well as some other Form
methods) call methods like:
$(form).getElements() instead of Form.getElements(form)
This will only work if the element is actually a form. Would anyone be
averse to going back to the old behaviour? It would simply mean
reverting back to things like Form.getElements instead of
$(form).getElements.
You could argue that you should only be using the Form methods on real
forms, but in practice it was useful not to have that restriction.
Thoughts?
-Jonathan.
Christophe Porteneuve
jonatha...@gmail.com a écrit :
> Form.disable('my_area')
>
> This would disable all form elements in my_area, even though my_area
> was not a form. As of r5449, Form.disable (as well as some other Form
> methods) call methods like:
Well, it's handy, but counter-intuitive and misleading, IMHO. You can
do the same thing today like this:
$($('my_area').getElementsByTagName('form')).invoke('disable');
Sure, it's more verbose, but not humongously so, and at least it's a
no-surprise code. If you're working on a small DOM, you can risk lower
performance in order to achieve smaller code:
$$('#my_area form').invoke('disable');
> This will only work if the element is actually a form. Would anyone be
> averse to going back to the old behaviour? It would simply mean
> reverting back to things like Form.getElements instead of
> $(form).getElements.
Well, not so much reverting back as providing both behaviors, since it
would be unacceptable to get rid of the latter form (this would be
inconsistent with the whole remainder of the API, which provides the
extension mechanism so code looks more OOP-like).
> You could argue that you should only be using the Form methods on real
> forms, but in practice it was useful not to have that restriction.
It doesn't remove the feature; it just makes it slightly less
"surprising," which I see as fitting the POLS...
Now, that's just me. But reverting change on this specific case would
introduce inconsistency with no great benefit, I believe.
Just my two cents,
--
Christophe Porteneuve a.k.a. TDD
"[They] did not know it was impossible, so they did it." --Mark Twain
Email: t...@tddsworld.com
RobG
Christophe Porteneuve wrote:
[...]
> Well, it's handy, but counter-intuitive and misleading, IMHO. You can
> do the same thing today like this:
>
> $($('my_area').getElementsByTagName('form')).invoke('disable');
getElementsByTagName returns a collection, not an Array. The result
must be converted to a Prototype.js array first:
$A($('my_area').getElementsByTagName('form')).invoke('disable');
invoke('disable') doesn't work in Safari 2.
The way disable works in Prototye seems confusingly lengthy:
disable: function(form) {
form = $(form);
var elements = Form.getElements(form);
for (var i = 0; i < elements.length; i++) {
var element = elements[i];
element.blur();
element.disabled = 'true';
}
return form;
},
You'll notice it calls getElements, which is:
getElements: function(form) {
form = $(form);
var elements = new Array();
for (var tagName in Form.Element.Serializers) {
var tagElements = form.getElementsByTagName(tagName);
for (var j = 0; j < tagElements.length; j++)
elements.push(tagElements[j]);
}
return elements;
Which references still other methods, all of which is extremely
inefficient.
To disable all the controls in a form, all that is required is to
iterate over the form's elements collection and disable them, here is a
modified version:
disable: function(form) {
if (typeof form == 'string') form = $(form);
var elements = form.elements;
for (var i=0, len=elements.length; i<len; i++){
elements[i].disabled = true;
}
return form;
},
Hugely more efficient and not reliant on a hard-coded list of tag
names.
--
Rob
Christophe Porteneuve
> getElementsByTagName returns a collection, not an Array. The result
> must be converted to a Prototype.js array first:
Correct. Actually $ should handle being passed an array as a single
argument... I might submit a tested patch. Still, the second syntax is
valid. And we could also go:
$A($('my_area').getElementsByTagName('form')).each(Element.hide);
> invoke('disable') doesn't work in Safari 2.
Ah, there's no nativeExtension? Well then the syntax above should work.
> To disable all the controls in a form, all that is required is to
> iterate over the form's elements collection and disable them, here is a
> modified version:
>
> disable: function(form) {
> if (typeof form == 'string') form = $(form);
You can go Prototypish and just get rid of the test part here.
> var elements = form.elements;
> for (var i=0, len=elements.length; i<len; i++){
> elements[i].disabled = true;
The blur call was there for a reason (I assume some browser does not
properly disable on focused elements).
> }
> return form;
> },
Aside from my two notes above, your replacement seems fine. Why don't
you create a tested patch for it and file in a Trac ticket with it? If
there are issues we missed, Thomas or Sam will say so. Otherwise, it
will eventually get committed, and everyone will benefit.
Sincerely,
RobG
> RobG a écrit :
> > getElementsByTagName returns a collection, not an Array. The result
> > must be converted to a Prototype.js array first:
>
> Correct. Actually $ should handle being passed an array as a single
> argument... I might submit a tested patch. Still, the second syntax is
> valid. And we could also go:
>
> $A($('my_area').getElementsByTagName('form')).each(Element.hide);
Hiding an element is an entirely different thing to disableing it.
>
> > invoke('disable') doesn't work in Safari 2.
>
> Ah, there's no nativeExtension?
I don't know, I didn't bother to debug the issue. I started,
discovered the cryptic disable routine and started to fix that. I
looked at the invoke method and started to see much of the same - I ran
out of steam.
> Well then the syntax above should work.
Hiding form controls is not a substitute for disabling them.
>
> > To disable all the controls in a form, all that is required is to
> > iterate over the form's elements collection and disable them, here is a
> > modified version:
> >
> > disable: function(form) {
> > if (typeof form == 'string') form = $(form);
>
> You can go Prototypish and just get rid of the test part here.
I don't like that approach - why call $() when there is no need?
Presumably $() can return an array of forms, neither my function or the
original will cope with that - should the function be extended to deal
with it?
I'd prefer:
if (typeof form == 'string') form = document.forms[form];
Which also avoids IE's confusion that NAMES are IDs.
> > var elements = form.elements;
> > for (var i=0, len=elements.length; i<len; i++){
> > elements[i].disabled = true;
>
> The blur call was there for a reason (I assume some browser does not
> properly disable on focused elements).
Documentation might help.
--
Rob
Christophe Porteneuve
>> $A($('my_area').getElementsByTagName('form')).each(Element.hide);
>
> Hiding an element is an entirely different thing to disableing it.
My mistake, I meant "Form.disable", not "Element.hide".
> I don't know, I didn't bother to debug the issue. I started,
> discovered the cryptic disable routine and started to fix that. I
> looked at the invoke method and started to see much of the same - I ran
> out of steam.
I believed Prototype worked around Safari/Konqueror's lack of DOM
element prototype for this (as indicated by dom.js' lines 454-460 in the
current SVN; these lines have been there for a while...).
> Hiding form controls is not a substitute for disabling them.
Same reply as before.
> I don't like that approach - why call $() when there is no need?
I doubt the cost of a function call will be significantly greater than
the cost of the typeof operator on such a case. Even if it were, just
how many forms do you intend to have on one page?!
> Presumably $() can return an array of forms, neither my function or the
> original will cope with that - should the function be extended to deal
> with it?
You see? A tinor change ripples through the codebase, making it more
and more inconsistent. I believe keeping the whole set of DOM
extensions single-element-oriented is better, if only to allow chain
calling, a strong design decision that appeared a few weeks ago.
Plus, my simple proposed usage lines do cope with it by using each or
invoke...
> Which also avoids IE's confusion that NAMES are IDs.
Proper naming of forms and elements is actually a developer issue, not a
library issue (at least from my viewpoint).
> Documentation might help.
On its way, Rob, on its way. People are working hard on it almost as we
speak :-) But then, documentation is for usage, and will not dive into
the internals. If you want to know why there's a blur() before, search
through the history of patches: there's likely a fixed bug ticket that
accounts for it.
Martin Bialasinski
>
> RobG a écrit :
> Aside from my two notes above, your replacement seems fine. Why don't
> you create a tested patch for it and file in a Trac ticket with it?
Usage of the elements collection of the Form element has already been
rejected by Thomas in favor of the current implementation.
Unfortunately, the reasoning in favor for the current implementation
is not documented
http://dev.rubyonrails.org/ticket/4249
> If there are issues we missed, [...] Sam will say so.
That would be a first.
RobG
Christophe Porteneuve wrote:
> RobG a écrit :
> > I don't like that approach - why call $() when there is no need?
>
> I doubt the cost of a function call will be significantly greater than
> the cost of the typeof operator on such a case. Even if it were, just
> how many forms do you intend to have on one page?!
The point is that even though a better method is available for
resolving the argument passed to the function, $() is used purely by
convention (see below).
>
> > Presumably $() can return an array of forms, neither my function or the
> > original will cope with that - should the function be extended to deal
> > with it?
>
> You see? A tinor change ripples through the codebase, making it more
> and more inconsistent.
I don't think consistency is the issue. If you want a certain set of
functions to accept only a single element, then make that well known
either in the call or the documentation. Then you can say something
like "all the Element methods accept only a single element as an
argument" or such.
How you resolve parameters within a function should be based on what is
best on a case-by-case basis, "consistency" might be a criterion but
not the only one.
> I believe keeping the whole set of DOM
> extensions single-element-oriented is better
Which seems to be implied but not stated. How does someone using
Prototype know whether they are using a DOM extension or not? Are they
just the ones prefixed by Element?
> if only to allow chain
> calling, a strong design decision that appeared a few weeks ago.
So the point of using $() is purely to resolve whether a form element
or a form name/id has been passed to the function. In that case, the
use of a typeof test followed by use of the forms collection is faster
and doesn't have problems with IE's name/id confusion.
It also indicates to anyone scanning the code (probably because they
got an error after passing an array) that the function only accepts a
form or form name/id.
In contrast, $() is less helpful than the above. It gives the
appearance that the function accepts an array of names, ids or forms
when it doesn't and doesn't help with IE's confusion with IDs and
NAMEs.
> > Which also avoids IE's confusion that NAMES are IDs.
>
> Proper naming of forms and elements is actually a developer issue, not a
> library issue (at least from my viewpoint).
"Proper"? There is nothing improper about having some elements with a
name that is the same as some other element's ID. That issue arises
from a design flaw in IE and can easily be avoided. It continues to be
an issue with Prototype because it insists on using $() rather than the
safer (faster and more widely supported) forms collection where forms
are involved.
It is very common for HTML to be authored separately from script,
Prototype should do whatever it can to support valid markup rather than
setting traps for the unwary.
> > Documentation might help.
>
> On its way, Rob, on its way. People are working hard on it almost as we
> speak :-) But then, documentation is for usage, and will not dive into
> the internals.
Documentation is for whatever purpose you wish it to serve. Any script
in a web page will have its internals inspected; where it is intended
to be distributed as a library for others to use, suitable comments
should be included (see below).
> If you want to know why there's a blur() before, search
> through the history of patches: there's likely a fixed bug ticket that
> accounts for it.
It is normal to comment such exceptions in the code, or at least in
separate developer documentation, because it is extremely likely that
anyone coming along afterwards will question why it's been used. Even
the original developer is likely to not remember the precise version of
whatever browser, or even which one, if indeed that is the reason (and
we don't know that it was).
If browser-friendliness is the issue, consider:
if (elements[i].blur) elements[i].blur();
--
Rob
RobG
Martin Bialasinski wrote:
> On 12/10/06, Christophe Porteneuve <t...@tddsworld.com> wrote:
> >
> > RobG a écrit :
>
> > Aside from my two notes above, your replacement seems fine. Why don't
> > you create a tested patch for it and file in a Trac ticket with it?
>
> Usage of the elements collection of the Form element has already been
> rejected by Thomas in favor of the current implementation.
> Unfortunately, the reasoning in favor for the current implementation
> is not documented
>
> http://dev.rubyonrails.org/ticket/4249
Changing the elements nodeList to an Array means that the collection is
no longer live - whether that was intended is another unknown. At
least your suggested method preserves that side-effect with the minimum
of fuss.
Anyone wanting a live nodeList is back to using the DOM elements
collection, or could use getElementsByTagName if that provides a
suitable subset.
I can't see any benefit in using getElement. Not only is it very
inefficient, it also mangles the order of the elements in a way that
may not be consistent across browsers - the properties of an object
(such as when using for..in) do not have to be returned in any
particular order. It has been an issue in the past where a particular
order was expected, but some browsers returned them in the order they
are added and others in some other order (say sorted alphabetically).
--
Rob
Christophe Porteneuve
> I don't think consistency is the issue. If you want a certain set of
> functions to accept only a single element, then make that well known
> either in the call or the documentation. Then you can say something
> like "all the Element methods accept only a single element as an
> argument" or such.
How about this in the CHANGELOG for rc1:
"For consistency, Element.toggle, Element.show, Element.hide,
Field.clear, and Field.present no longer take an arbitrary number of
arguments."
There are also other consequences and related behaviors detailed in the
CHANGELOG for rc1 and rc2.
> Which seems to be implied but not stated. How does someone using
> Prototype know whether they are using a DOM extension or not? Are they
> just the ones prefixed by Element?
And Form.Element, yes. As for knowing about it, it does require reading
some docs, most of which are absent so far, I agree. But that issue is
going to be fixed pretty soon.
> So the point of using $() is purely to resolve whether a form element
> or a form name/id has been passed to the function. In that case, the
It's also to guarantee the resulting form is extended with Form.* methods.
> use of a typeof test followed by use of the forms collection is faster
> and doesn't have problems with IE's name/id confusion.
Again, name/id confusion wouldn't occur if the markup didn't confuse
both. These attributes have entirely distinct semantics. Should they
be used on the same element with the same value, as is too often the
case, that won't be a problem either. But using same-value ID and NAME
on distinct elements is misguided IMHO.
> It also indicates to anyone scanning the code (probably because they
> got an error after passing an array) that the function only accepts a
> form or form name/id.
Actually, the error message you get, which varies from browser to
browser, can be pretty obscure :-(
> "Proper"? There is nothing improper about having some elements with a
> name that is the same as some other element's ID. That issue arises
Ah. Our philosophies of semantics in a web page differ then. Although
I can see contexts in which your statement is obviously valid, but I'll
always use workarounds (e.g. prefixing IDs when there is such an
impending conflict).
> Documentation is for whatever purpose you wish it to serve. Any script
> in a web page will have its internals inspected; where it is intended
> to be distributed as a library for others to use, suitable comments
> should be included (see below).
You'll find a lot of people stating that inline-doc'ing a library
intended for such wide use will bump up its download size by a factor of
2x or 3x... So it should have to remain in some ref' version of the
script, which is better done with separate doc altogether, I guess.
This latter approach has the advantage of being more accessible to
entry-level web developers, which are legion.
Martin Bialasinski
> Changing the elements nodeList to an Array means that the collection is
> no longer live - whether that was intended is another unknown.
Yes. Prototype never returns a NodeList but an (automatically
Enumerable extended) Array.
> the properties of an object (such as when using for..in) do not have to be
> returned in any particular order.
> It has been an issue in the past where a particular
> order was expected, but some browsers returned them in the order they
> are added and others in some other order (say sorted alphabetically).
I know that the order in "for..in" is an implementation details and I
remember at least one other place in Prototype where in = out is
assumed. Browser implementations seem to respect the order as far as I
can say, so I'd love to hear about the browser you mention which does
not do so. Which one is it?
RobG
> On 12/11/06, RobG <rg...@iinet.net.au> wrote:
>
> > Changing the elements nodeList to an Array means that the collection is
> > no longer live - whether that was intended is another unknown.
>
> Yes. Prototype never returns a NodeList but an (automatically
> Enumerable extended) Array.
I'm rapidly coming to the conclusion that Prototype is not for me.
>
> > the properties of an object (such as when using for..in) do not have to be
> > returned in any particular order.
> > It has been an issue in the past where a particular
> > order was expected, but some browsers returned them in the order they
> > are added and others in some other order (say sorted alphabetically).
>
> I know that the order in "for..in" is an implementation details and I
Yes, ECMAScript Language Specifiction, Ed 3, Section 12.6.4.
"The mechanics of enumerating the properties ... is implementation
dependent. The order of enumeration is defined by the object."
> remember at least one other place in Prototype where in = out is
> assumed. Browser implementations seem to respect the order as far as I
> can say, so I'd love to hear about the browser you mention which does
> not do so. Which one is it?
Opera 7, though by version 7.5 it seems they'd reverted to the same
ordering as other browsers (that is, the order in which they are
added). That seems to hold for user defined native objects, but not
for built-in objects. Try the following in various browsers, in
Firefox 2.0 on Mac the order of the added properties is preserved if a
native object is used, but reversed if the window object is used:
<div id="xx"></div>
<script type="text/javascript">
function foo(obj) {
var props = {alpha:'alpha', gamma:'gamma',
delta:'delta', beta:'beta'};
for (var p in props) obj[p] = props[p];
var x = [];
for (p in obj){
if (p in props){
x.push(p);
}
}
return x;
}
window.onload = function(){
document.getElementById('xx').innerHTML = foo(this).join('<br>');
}
</script>
Thanks for the heads-up on Prototype's use of for..in - it is poor
design to to rely on conformance with behaviour that is not supported
by the ECMAScript standard and where examples of standards-compliant
non-conformance can be demonstrated in common, modern browsers.
There is a thread here on comp.lang.javascript that may be of interest:
--
Rob
Peter Michaux
> intended for such wide use will bump up its download size by a factor of
> 2x or 3x...
Both inline comments and helpful use of whitespace in a script fall
into the same category of inflating the script in favor of
understandability. That doesn't mean the comments or whitespace need
to be downloaded. If semicolons, braces and care with things like + ++
is used then the file can be minimized with jsmin.
<http://yuiblog.com/blog/2006/10/16/pageweight-yui0114/>
Peter
-------
http://forkjavascript.org
Martin Bialasinski
> > Yes. Prototype never returns a NodeList but an (automatically
> > Enumerable extended) Array.
>
> I'm rapidly coming to the conclusion that Prototype is not for me.
This behaviour is what I need 100% of cases, so I works great for me.
Nevermind that some methods like the CSS-style-selectors would
probably be impossible to implement in a way that returns a NodeList.
I prefer consistent return data.
> Try the following in various browsers, in
> Firefox 2.0 on Mac the order of the added properties is preserved if a
> native object is used, but reversed if the window object is used:
Thanks for the info.
Jonathan Viney
Something like $$('#my_area form') will not work because I sometimes don't use form tags (for various reasons that I won't go into). At the most basic level, I've got a bunch of input elements inside a container and I want to enable/disable all of them, whether or not they are in a form is neither here nor there.
The change in r5449 to work only with real form elements was probably more significant than was realised at the time. I used to be able to use Form.enable/disable more effectively, Form.serialize could also serialize the form elements in any container (useful in places forms are not allowed, like around table rows).
I can do the following to get the old behaviour back, but it's not very nice.
var Container = Object.keys(Form.Methods).inject({}, function(container, method) {
container[method] = function() {
var args = $A(arguments), element = $( args.shift());
var cache = Element.extend.cache, methods = Object.clone(Form.Methods);
for (var property in methods) {
var value = methods[property];
if (typeof value == 'function' && !(property in element))
element[property] = cache.findOrStore(value);
}
return Form[method].apply(null, [element].concat(args));
};
return container;
});
Now Container can be used instead of Form if you're not dealing with form elements. Any other suggestions?
-Jonathan.
RobG
> On 12/11/06, RobG <rg...@iinet.net.au> wrote:
> >
> > Martin Bialasinski wrote:
> > > Yes. Prototype never returns a NodeList but an (automatically
> > > Enumerable extended) Array.
Not always - check Form.Methods.getInputs:
getInputs: function(form, typeName, name) {
form = $(form);
var inputs = form.getElementsByTagName('input');
if (!typeName && !name)
return inputs;
...
Seems to me that will return a NodeList/HTMLCollection, since that's
what getElementsByTagName returns.
<URL: http://www.w3.org/TR/DOM-Level-2-Core/core.html#ID-A6C9094 >
If you provide values for type or name (or both) you'll get an Array.
> > I'm rapidly coming to the conclusion that Prototype is not for me.
>
> This behaviour is what I need 100% of cases, so I works great for me.
You'd better submit a change then.
--
Rob
donald...@gmail.com
the documentation for link_to_remote (and as a consequence also for the
form_remote_tag, etc..).
What i'm talking about is the :submit property, which the documentation
defines as:
:submit: Specifies the DOM element ID that's used as the parent of
the form elements. By default this is the current form, but it could
just as well be the ID of a table row or any other DOM element.
Well unfortunately with the latest change this is not true anymore, so
either somebody should change the documentation to reflect this, or
update the code to somehow make it work again.