what's wrong with this code?

seeker · Dec 9, 2005

Hi,

Does anybody know what's wrong with the following code:

<div align="center"><a href="flashplay.php"
onclick="NewWindow(this.href,'Listen
this!','280','280','no','random');return false" onfocus="this.blur()"
onMouseOut="MM_swapImgRestore()"
onMouseOver="MM_swapImage('Image20','','gifs/mp3_1.gif',1)"><img
src="gifs/mp3.gif" name="Image20" width="125" height="38"
border="0"></a></div>

The intention is this: when someone click on a rollover image, a new window
pop's up displaying the flashplay.php 280x280px page, but for some reason it
doesn't work...If anyone knows a solution, I'd appreciate it a lot.

unixfreak0037 · Dec 9, 2005

We'd need to see the source code to your "NewWindow" function before
anyone can help. But also 1) are you blocking popups and 2) are you
sure you should even be using popups? Most people I know block them.

seeker · Dec 9, 2005

We'd need to see the source code to your "NewWindow" function before
anyone can help. But also 1) are you blocking popups and 2) are you
sure you should even be using popups? Most people I know block them.

Sorry, my bad.

The source code included in the <head> tag is as follows:

<script language="javascript" type="text/javascript">

</script>

Regarding the issues 1) and 2) - no, I'm not blocking pop-ups and 2)I am
sure I want to use popups. Not that I like them, but I have my objective
reasons.

Thomas 'PointedEars' Lahn · Dec 9, 2005

seeker said:
The source code included in the <head> tag is as follows:

<script language="javascript" type="text/javascript">

</script>

That code is utter nonsense. Do not use it.

PointedEars

Michael Winter · Dec 9, 2005

On 09/12/2005 14:30, seeker wrote:

[snip]

<script language="javascript" type="text/javascript">
<!--

The language attribute is deprecated and redundant, and the SGML
comments are unnecessary, too.

var win=null;
function NewWindow(mypage,myname,w,h,scroll,pos){
if(pos=="random"){LeftPosition=(screen.width)?Math.floor(Math.random()*(scre
en.width-w)):100;TopPosition=(screen.height)?Math.floor(Math.random()*((scre
en.height-h)-75)):100;}
if(pos=="center"){LeftPosition=(screen.width)?(screen.width-w)/2:100;TopPosi
tion=(screen.height)?(screen.height-h)/2:100;}
else if((pos!="center" && pos!="random") ||
pos==null){LeftPosition=0;TopPosition=20}
settings='width='+w+',height='+h+',top='+TopPosition+',left='+LeftPosition+'
,scrollbars='+scroll+',location=no,directories=no,status=no,menubar=no,toolb
ar=no,resizable=no';
win=window.open(mypage,myname,settings);}

The code you have there is horrid. The following is better:

function createChromelessWindow(uri, name, width,
height, centre)
{
var features = 'resizable,scrollbars',
popup;

if(('function' == typeof window.open)
|| ('object' == typeof window.open))
{
if(!isNaN(width) && !isNaN(height)) {
features += ',width=' + width + ',height=' + height;

if(centre && ('object' == typeof screen)
&& !isNaN(screen.height)
&& !isNaN(screen.width))
{
features += ',left=' + ((screen.width - width) / 2)
+ ',top=' + ((screen.height - height) / 2);
}
}
popup = window.open(uri, name, features);
}
return (popup && (window != popup)) ? popup : null;
}

The random placement feature has been removed as it's a stupid idea, and
centring is controlled via an (optional) boolean. Scrollbars and
resizing are enabled as there is /never/ a reason not allow them.
Finally, rather than assigning the reference to the popup to a global
variable, the reference is returned.

[snip]

All that said, it has nothing to do with your problem. That is almost
certainly caused because of the name you tried to give the new window.
The use of 'Listen this!' is a very bad idea. The name should be
restricted to alphanumeric characters only, contain no spaces,
punctuation or other characters, and must begin with a letter. Something
simpler like 'flashplayer' (based on your URI) would be far better.

<a href="flashplay.php" target="flashplayer"
onclick="return !createChromelessWindow(this.href,
this.target, 280, 280);"
...>...</a>

Mike

Thomas 'PointedEars' Lahn · Dec 9, 2005

Michael said:
The code you have there is horrid. The following is better:

function createChromelessWindow(uri, name, width,
height, centre)
{
var features = 'resizable,scrollbars',
popup;

if(('function' == typeof window.open)
|| ('object' == typeof window.open))
{
[...]
popup = window.open(uri, name, features);
}
return (popup && (window != popup)) ? popup : null;

IMO `window' can only refer to the same object as `popup' if the value of
`name' is the name of the current window. Why would you want to return
`null' then, as if creating the popup failed? It did not really fail,
the location of the target window was replaced with the value of `uri'.

}

The random placement feature has been removed as it's a stupid idea, and
centring is controlled via an (optional) boolean.

I have that boolean in my window.js as well because it was written before
I realized that "center" does not necessarily mean the center of one screen.
(Thanks to kind people in de.comp.lang.javascript and here that provided
photos.)

Especially, it has been reported here that IE always returns the values for
the primary display, while Firefox returns the values for the screen the
(left top corner of the) accessing window is located in.

I have the general use of that argument marked as deprecated since then.

PointedEars

Michael Winter · Dec 9, 2005

Michael Winter wrote:
[snip]

return (popup && (window != popup)) ? popup : null;

Click to expand...

IMO `window' can only refer to the same object as `popup' if the value of
`name' is the name of the current window.

That wasn't something that I had considered.

Why would you want to return `null' then, as if creating the popup failed?

It is one known pop-up blocking technique to return a reference to the
current window, as in:

window.open = function() {
return window;
};

Though I had no intention of trying to detect failure in general
(impossible anyway), I thought it beneficial to cope with some of the
more simple circumstances.

The code should revert to the initial version: the popup variable should
be removed, the return statement (as it currently is) should also be
removed, and the assignment to 'popup' should become a return statement.

[snip]

I have that boolean in my window.js as well because it was written before
I realized that "center" does not necessarily mean the center of one screen.

I realise that, too. However, I let it remain as a 'less stupid' feature
from the original code.

I am of the opinion that the window manager should be left to its job,
and would personally never attempt to centre a pop-up. I'd rarely use a
pop-up, full stop, but that's a different matter.

[snip]

Mike

What's wrong with this code?	10	Jul 23, 2012
Help with Visual Lightbox: Scripts	2	May 3, 2023
what's wrong with this code?	2	Sep 11, 2009
I can't figure this out! Please tell me what's wrong!	4	Aug 10, 2006
What's wrong with this code??	11	Mar 16, 2007
What's wrong with this code snippet?	9	Jan 4, 2006
What's wrong with this code?	6	Jul 2, 2005
What's wrong with this server code ?? Cant read incoming data	6	Jan 23, 2007

what's wrong with this code?

seeker

unixfreak0037

seeker

Thomas 'PointedEars' Lahn

Michael Winter

Thomas 'PointedEars' Lahn

Michael Winter

Ask a Question

Similar Threads

Members online

Forum statistics

Latest Threads