[PATCH] JS Form Popups

[David Faure]

On Tuesday 03 June 2003 21:01, Ralf Hoelzer wrote:
> This patch that tries to fix Bug #58650:
> 
> http://bugs.kde.org/show_bug.cgi?id=58650
> 
>  If a JavaScript form.submit() is called and the form has a non-existent 
> target specified, a new browser window pops up. This trick to get around 
> popup blockers is used
> on http://sharereactor.com. 
> 
> The patch tries to check if a frame with the target name exists. If not, it 
> either blocks it (deny/smart policy) or asks. I've put it into ecma/kjs_html 
> to make sure it's only checked if the form is submitted via JS.
> 
> I tested this with a few testcases and it works for me. I wasn't sure if a 
> frame is allowed to be replaced in a different frameset. That check might 
> need to be added.

Thanks for the patch.
I think it could be improved a little.

A method to factorize the KMessageBox question would be a good idea, to
avoid code duplication and facilitate future maintainance. Although... I wonder
if the message shouldn't be a bit different, to explain that answering "no" will
NOT submit the form? It's a bit different from "no popups please" - here the
form will not be posted, which in some cases is much worse than the resulting
popup...

Maybe it would also be clearer (and safer) if the if() that tests the windowopenpolicy
was the very first thing, around all this. Just to make sure we don't affect the 
behaviour in the other cases due to some mistake in this code (in the future :).

It also feels wrong to have an if( no target ) submit() _after_ asking the question
to the user. Couldn't this result in submitting even though the user said no?

As I see it the logic should be:
if ( policy is to ask )
{
 do all checks to see if we need to ask;
  if (we need to ask) {
     ask();
      -> if the user chose no, return.
  }
}
submit.   << the only call to submit, in fact.

-- 
David FAURE, faure at kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).
Qtella users - stability patches at http://blackie.dk/~dfaure/qtella.html