Syntax Error

[aax123]

Can someone tell me what's wrong with this? I think it has to do with my placement of &

window.open('/liners/print_liner.aspx?Liner='+lid&sortOrder='+sortOrder&sorDir='+sortDir ,'Print_liner','toolbar=no,directories=no,menubar=no,status=no,scrollbars=yes,resizable=no,width=350,height=250,screenX=10,screenY=10,left=0,top=0');

[Bernard Marx]

Hi aax,

Not sure which are the JS variables, and which are literals.
Perhaps you can sort it out yourself with some syntax highlighting.

window.open[red]([/red]
    [blue]'/liners/print_liner.aspx?Liner='[/blue][red]+[/red]lid[red]&[/red]sortOrder[red]=[/red][blue]'+sortOrder&sorDir='[/blue][red]+[/red]sortDir[red],[/red]
    [blue]'Print_liner'[/blue][red],[/red]
    [blue]'toolbar=no, etc...'[/blue]
[red]);[/red] 

[rocknbil]

for starters, look at your quoting, and this corrected version:

'/liners/print_liner.aspx?Liner='+lid+'&sortOrder='+sortOrder+'&sorDir='+sortDir

At this point, it looks like "lid" is undefined, and you've got some confusion between variables and actual strings, as well as you probably intended "sortDir" instead of "sorDir." But setting those aside, to make it easier to maintain I would store the string in a variable

var url = '/liners/print_liner.aspx?Liner='+lid+'&sortOrder='+sortOrder+'&sortDir='+sortDir;

then

window.open(url,'Print_liner','toolbar=no,directories=no,menubar=no,status=no,scrollbars=yes,resizable=no,width=350,height=250,screenX=10,screenY=10,left=0,top=0');

While you're at it, a few other things to make it easier to maintain. You don't need to specify no, or even yes, for the parameters. if you want something in, just include the param, if not, leave it out:

window.open(url,'Print_liner','scrollbars,width=350,height=250, screenX=10,screenY=10,left=0,top=0');

Second, using the window name 'Print_liner' will insure that any subsequent clicks will ALWAYS open in the same window, the one you've named Print_liner. The problem here is that if someone clicks back in the main window and clicks another new window link, it will load in the same window (now invisible because it's behind the main window) and the impression is that nothing happens. So a simple unique ID (see below) will insure a new window opens for every instance.

Last, I recommend always including resizable as you never know the user's environment, leave them the option. I don't know how many developers' ears are burning from my ranting when trying to perform a task on their sites. :-)

So the final tweaked code:

var day = new Date();
var id = day.getTime();
var url = '/liners/print_liner.aspx?Liner='+lid+'&sortOrder='+sortOrder+'&sortDir='+sortDir;

// I even like to do this . . .
var params = 'scrollbars,resizable,width=350,height=250,screenX=10,screenY=10,left=0,top=0';

window.open(url,id,params);

ARG! Sorry B.M., too slow on the draw today . . .

[aax123]

Thanks so much.