Development

#4152 (Confirm popup make javascript call don't work with IE)

You must first sign up to be able to contribute.

Ticket #4152 (closed defect: fixed)

Opened 6 years ago

Last modified 5 years ago

Confirm popup make javascript call don't work with IE

Reported by: Leonard Assigned to: FabianLange
Priority: major Milestone: 1.2.9
Component: helpers Version: 1.1.0
Keywords: link_to_function, javascript, IE, confirm, popup Cc:
Qualification: Ready for core team

Description

While using a remote_function, link_to_function, link_to remote or any function that uses a 'confirm' option, the call don't work on IE6 and IE7

the generated code is "... onClick="if(confirm('Confirmation message'){..." whereas it should be "... onClick="if(window.confirm('Confirmation message'){..."

Attachments

window_confirm.diff (0.6 kB) - added by Intru on 07/10/09 15:56:48.

Change History

(in reply to: ↑ description ) 08/07/08 17:34:06 changed by Leonard

  • status changed from new to closed.
  • resolution set to fixed.

Replying to Leonard:

While using a remote_function, link_to_function, link_to remote or any function that uses a 'confirm' option, the call don't work on IE6 and IE7 the generated code is

"... onClick="if(confirm('Confirmation message'){..."

whereas it should be

"... onClick="if(window.confirm('Confirmation message'){..."

PATCH

File : php5\PEAR\symfony\helper\JavascriptHelper?.php

line 94 is :

$html_options['onclick'] = "if(confirm('$confirm')){ $function;}; return false;";

line 94 should be :

$html_options['onclick'] = "if(window.confirm('$confirm')){ $function;}; return false;";

line 482 is :

$function = "if(confirm('".escape_javascript($options['confirm'])."')) { $function; }";

line 482 should be :

$function = "if(window.confirm('".escape_javascript($options['confirm'])."')) { $function; }";

08/07/08 20:27:08 changed by Carl.Vondrick

Is this fixed? or is just ready for review?

08/09/08 23:23:45 changed by Leonard

  • status changed from closed to reopened.
  • resolution deleted.

I don't know. I've only posted the ticket and proposed a fix because I thaught this would interest other symfony users.

I don't know if the Symfony team have taken this into account.

08/11/08 08:53:53 changed by FabianLange

adding the window namespace explicitly should do no harm. imho we can apply this for 1.x

04/17/09 02:21:00 changed by Andromeda

Why is this fix in 1.2.5 still not applied? Would really help me... and probably also others who have to fight with IE6.

04/17/09 03:10:42 changed by dwhittle

  • status changed from reopened to closed.
  • resolution set to fixed.

(In [17383]) [1.3] fixed confirm dialog does not work in ie6 (closes #4152)

04/17/09 03:11:05 changed by dwhittle

(In [17384]) [1.2] fixed confirm dialog does not work in ie6 (closes #4152)

04/17/09 03:11:23 changed by dwhittle

(In [17385]) [1.1] fixed confirm dialog does not work in ie6 (closes #4152)

04/17/09 03:11:42 changed by dwhittle

(In [17386]) [1.0] fixed confirm dialog does not work in ie6 (closes #4152)

(follow-up: ↓ 12 ) 04/17/09 03:12:29 changed by dwhittle

  • milestone set to 1.2.6.

04/17/09 03:12:36 changed by dwhittle

  • qualification changed from Unreviewed to Ready for core team.

(in reply to: ↑ 10 ) 06/28/09 13:43:37 changed by Intru

  • keywords changed from link_to_remote, link_to_function, remote_function, javascript, IE, confirm, popup to link_to_function, javascript, IE, confirm, popup.
  • status changed from closed to reopened.
  • resolution deleted.

Shouldn't this patch also be applied to source:/branches/1.2/lib/helper/JavascriptBaseHelper.php@trunk#L47 in order to fix link_to_function()? The patch in [17384] only fixes it for remote_function()

07/10/09 15:56:48 changed by Intru

  • attachment window_confirm.diff added.

08/06/09 21:37:10 changed by boutell

  • priority changed from minor to major.

window.confirm is a band-aid... the real problem is that link_to_function adds the onClick attribute but does not remove confirm from html_options. As a result, a confirm attribute shows up in the 'a' tag.

This has two problems:

1. There's no such attribute, it's invalid HTML. 2. In IE, the current tag is the first context in which a property matching a method name without an explicit object is looked for. When it finds the 'confirm' attribute and sees that it is not a method, it knows it can't execute it, and generates the error.

The correct fix in link_to_function:

if ( isset($html_optionsconfirm?) )

{

$confirm = escape_javascript($html_optionsconfirm?); $html_optionsonclick? = "if(confirm('$confirm')){ $function;}; return false;"; // tom@punkave.com: without this we break IE, // and produce invalid HTML in any case unset($html_optionsconfirm?);

}

08/06/09 21:39:01 changed by boutell

Let's see if I can format that code better with wikiformatting:

if ( isset($html_options['confirm']) )
    {
      $confirm = escape_javascript($html_options['confirm']);
      $html_options['onclick'] = "if(confirm('$confirm')){ $function;}; return false;";
      unset($html_options['confirm']);
    }

08/07/09 00:53:16 changed by FabianLange

  • owner changed from fabien to FabianLange.
  • status changed from reopened to new.
  • milestone changed from 1.2.7 to 1.2.9.

oh how could we miss that unset there.

08/07/09 00:54:45 changed by FabianLange

  • status changed from new to closed.
  • resolution set to fixed.

(In [20870]) [1.2, 1.3] fixed invalid confirm attribute in html showing up by not unseeting the confirm attribute passed into javascript functions. (fixes #4152)