How Not to Solve a Problem
David Mark
in two places:-
// Safari mis-reports the default selected property of a hidden option
// Accessing the parent's selectedIndex property fixes it
if ( name === "selected" && elem.parentNode ) {
elem.parentNode.selectedIndex;
...to "fix" a part of their code that has trouble determining a
selected OPTION element.
http://groups.google.com/group/jquery-dev/browse_thread/thread/fe658871a8bff208#
It turned out that the issue was with elements that had not yet been
added to the DOM (or that were just added during parse and not quite
ready yet. So this really is a non-issue.
Regardless, the first thought that came to mind for determining the
selected OPTION of a SELECT was selectedIndex. Unsurprisingly, that
stuck. :)
The second thought that came to mind was that the parent of an OPTION
could be any number of number of things.
So it was mentioned to the jQuery liaison that perhaps checking the
selectedIndex property would be a bit more direct than _evaluating_
the property (and discarding the relevant value). Seems like some
implementations may treat the latter as a noop; but regardless, such
evaluations are always last-ditch end-arounds.
http://groups.google.com/group/jquery-dev/browse_thread/thread/fe658871a8bff208#
"After some discussion with colleagues, a better approach has been
identified. However, I would suggest that this simply be removed from
the code. The specs do not demand the behavior being sought after, and
the correct solution to the "problem" would be to simply add a
"selected" attribute to the first
<option>.
If you still want this "fix" in there, then I suggest adding this
method:
// Get the true "selected" property of an option.
// Safari mis-reports the selected property of the first (default)
selected
// option in a single-select list, when checked before the window's
onload
// event has fired.
function getOptionSelected(el) {
// Do a feature test to see if the "broken" functionality exists
var s = document.createElement("select");
s.add(new Option("a"));
if (s.options[0].selected) {
// Not broken, so unroll this method to be efficient next time
return (getOptionSelected = function(o) {return o.selected;})(el);
}
s=null;
// If we're getting a false value, it may be that the option
// will be selected when fully rendered, but has not happened yet.
return (getOptionSelected=function(o) {
if (o.selected === false) {
var s=(o.parentNode.tagName=="SELECT")?
o.parentNode:o.parentNode.parentNode;
// Verify selectedIndex because this doesn't apply to multiple-
selects
if (s.selectedIndex>=0) {
s.options[s.selectedIndex].selected=true;
}
}
return o.selected;
})(el);
}
Looks like one winner, one loser. In reality, it's two losers. This
is the one salient line (and it's botched):-
s.options[s.selectedIndex].selected=true;
Now, call me crazy, but calling a method to read the value of a
property should not _set_ that property. That's not its job and what
would be the point anyway? Just return true if the selectedIndex
matches the OPTION indicated. ;)
Here's the new "fix" that "landed" as a response:-
if ( name === "selected" && parent ) {
parent.selectedIndex;
// Make sure that it also works with optgroups, see #5701
if ( parent.parentNode ) {
parent.parentNode.selectedIndex;
Really?! ISTM that a get operation should not evaluate properties of
_other nodes_ either. And this doesn't address the underlying problem
at all (and neither do the accompanying comments describe it). It's
just as much a magic spell now as before (technically two spells now).
And I thought they had an embargo on any advice that might be
advocated in a CLJ post anyway. No, that's not a joke. They really
do think that way. :)
"As it is the overhead to the existing fix is very minimal, has no
function calls, no DOM manipulation, and is only 6 lines of code (even
with the optgroup fix)."
So, having the code that make sense was less of a concern than an
extra six lines (or so) of code. Actually, he added some feature test
too, so they are likely about the same. Even for those with no
knowledge of JS or browser scripting at all, this is clearly
fallacious reasoning. And these "retorts" are always chirped with
such breathless enthusiasm. Gee, only six lines of code! Not that it
matters, but how many lines will the minified result have? ;)
"Doing a bit more research I found a very similar problem when you use
dynamically-created optgroups in IE. I found a feature detect that was
able to detect the specific logic in Webkit and IE and skip the
property accesses if not necessary."
If he only realized that's not a bug either. Add it to the document
and its a bug. The "property accesses" are not the solution in any
event. How clueless would you have to be to not check the
selectedIndex property. IIRC, Matt tried to resist the idea at first
(i.e. what would _that_ have to do with it--it's always _right_; oh
right.) But after a few more exchanges, it sort of clicked and here
we are (nowhere).
"> Again, though, my suggestion is to just remove it. I don't think
> generalized library code should try to normalize behavior that is not
> demanded by the specs and that can be easily corrected through proper
> HTML."
Yes.
"Although, I seem to remember having a very heated discussion wherein
you advocated that our position of asking users to specify a doctype
to animate element width/height to 0 in IE 6 was too extreme and that
we shouldn't be making demands of a user's markup. (Granted that is
likely the only place where we make such a demand: We just ask the
users to provide a document that validates first.)"
No. Quirks mode would seem one reason for the existence of such
scripts (e.g. "smoothing out" differences in box models). We've seen
what their - height - method looks like (width() can't be far off).
They only "support" one box model (and poorly at that). Specifying a
doctype may or may not prevent quirks mode, which may or may not
change the box model used to render a random element. And none of
this has anything at all to do with validation. The "issue" at hand
boils down to whether the element is part of a document and whether
the DOM is ready (the latter is another subject they are lost at).
"Thanks for the pointers. In the future, filing individual
tickets/patches/test cases will be quite helpful."
Definitely not. Filing patches is not helping this guy (or anyone
else) one iota. And there's still one of these left to "fix". :(
What is the real "supported" list for this atrocity at this point?
Approximately:-
FF3(.5?), Safari 4, Chrome, Opera 10 and IE8 (standards mode only,
please).
Anything else and it might work. Or it might render the document
unusable. The typical jQuery user couldn't tell either way and
there's nothing they could do about it anyway as jQuery makes
progressive enhancement _impossible_. How would apps know if a jQuery
method will work or not? As there is no way to tell, they just call
them and hope for the best. :)