Gérard Talbot said:
tochiromifune wrote :
Why create an extra local variable?
It would be a global, not local one. There is no reason why besides missing
knowledge; the author should have used the `var' keyword to make it local:
function ...(...)
{
var selectedCity = town;
}
One valid reason to copy an argument's value this way is to modify the value
but use the argument's original value later. However, that does not happen
here; so the line is redundant, indeed.
You first need to check the existence of the window reference and its
closed prroperty.
Absolutely true. And `theURL' and `newWindow' should be declared locals
(at least `theURL' should), where `theURL' is in fact redundant:
var newWindow = window.open(
"seetown.php?select=" + selectedCity,
"infoWin",
"width=800,height=600");
This is of course still potentially harmful: there is no guarantee that the
window can be opened with that dimensions or that they make sense anyway.
Especially, the new window would lack scrollbars and the feature of
resizability which in combination is certainly a Bad Thing. And all URI
components should be properly escaped. So I propose
function isMethodType(s)
{
return (s == "function" || s == "object");
}
var esc = (isMethodType(typeof encodeURIComponent)
? encodeURIComponent
: (isMethodType(typeof escape)
? escape
: function(s) { return s; }));
var newWindow = window.open(
"seetown.php?select=" + esc(selectedCity),
"infoWin",
"resizeable,scrollbars");
instead. The size of the new window is determined by the UA, which should
follow what the user configured; it is resizable by the user and has
scrollbars in case the dimensions of the desktop do not suffice for the
user to resize the window large enough. And the URI component is properly
escaped.
That's not the correct way to call the focus() command.
Actually, it is a method that is called. And it is a way I would consider
to be correct (it is not logical to assume that if one Window object has
the `focus' property, another, newly created one, would have not -- or have
you empirical proof of the opposite?), although the feature test allows for
improvement.
I would write
if (newWindow
&& !newWindow.closed
&& isMethodType(typeof newWindow.focus))
{
newWindow.focus();
}
If the window is new, then, as you say yourself, you do not need to make
the focus() call.
Not true. If there was already a window with the same name ("infoWin"),
its content only has changed due to window.open() but it is unlikely to be
focused already. So it is a Good Thing to try to set the focus to it.
PointedEars