UpdateCc, clickable addresses bug?

Hi List,

I have the following problem in 3.8.6 (sorry I can’t try it on 3.8.7 now).
If I have a header line in an email like this (or in Cc field):To: “‘Foo Bar’” foo.bar@foobar.hu
of
To: ‘Foo2 Bar2’ foo2.bar2@foobar2.hu

on the Update.html page (UpdateCc component) RT generates the code:

<input

id=“UpdateCc-foo.bar@foobar.hu”
name=“UpdateCc-foo.bar@foobar.hu”

type="checkbox"
onClick="checkboxToInput('UpdateCc',

‘UpdateCc-foo.bar@foobar.hu’,‘"'Foo Bar'"
<foo.bar@foobar.hu lt%3Bfoo.bar@foobar.hu>’ );
$(UpdateIgnoreAddressCheckboxes).value=1"

 > Foo Bar

and a very similar one for foobar2 entry.
In that case if I click on one of those checkboxes, the address doesn’t
appear in the one-time-cc field.
I tried it with the same result in the latest version FF, IE, Opera and
Chrome.
In error console I can see the following error:

Error: missing ) after argument list
Source File: https://rt.foobar.hu/rt3/Ticket/Update.html?Action=Comment&id=Xhttps://rt.pontez.hu/rt3/Ticket/Update.html?Action=Comment&id=273
Line: 1, Column: 74
Source Code:
checkboxToInput(‘UpdateBcc’, ‘UpdateBcc-foo.bar@foobar.hu’,‘“‘Foo Bar’” <
foo.bar@foobar.hu>’ ); $(UpdateIgnoreAddressCheckboxes).value=1

It seems that ’ disturbs the generated JavaScript code.
Is it a known issue? Or a Bug? Or a Feature?

All help is greatly appreciated!
Bekeny

Hi List,

I have the following problem in 3.8.6 (sorry I can’t try it on 3.8.7 now).
If I have a header line in an email like this (or in Cc field):

To: “‘Foo Bar’” foo.bar@foobar.hu
of
To: ‘Foo2 Bar2’ foo2.bar2@foobar2.hu

on the Update.html page (UpdateCc component) RT generates the code:

[…]

It seems that ’ disturbs the generated JavaScript code.
Is it a known issue?

I didn’t know about it.

Or a Bug?

It sure looks like a bug. Will you open a ticket? A patch would be much
appreciated if you or anyone else is up to it.

Or a Feature?

It’s definitely not one of these.

-Jesse

signature.asc (197 Bytes)

Hi Jesse,

I attached a patch for 3.8.6.
I’m not sure it is the best solution for this problem but it seems a working
one.
Hope I helped.

Best regards,
Bekeny2009/12/17 Jesse Vincent jesse@bestpractical.com

On Thu 17.Dec’09 at 13:43:41 +0100, BALINT Bekeny wrote:

Hi List,

I have the following problem in 3.8.6 (sorry I can’t try it on 3.8.7
now).
If I have a header line in an email like this (or in Cc field):

To: “‘Foo Bar’” foo.bar@foobar.hu
of
To: ‘Foo2 Bar2’ foo2.bar2@foobar2.hu

on the Update.html page (UpdateCc component) RT generates the code:

[…]

It seems that ’ disturbs the generated JavaScript code.
Is it a known issue?

I didn’t know about it.

Or a Bug?

It sure looks like a bug. Will you open a ticket? A patch would be much
appreciated if you or anyone else is up to it.

Or a Feature?

It’s definitely not one of these.

-Jesse

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAksqQg0ACgkQEi9d9xCOQEbYEACeN5Wphc16WbGraqy9ayXsPeVt
uHYAn1I2sS4AHEqkKNeK4uY+TZRwgjao
=N12W
-----END PGP SIGNATURE-----

clean_address_in_RT_3_8_6.patch (1.78 KB)

Bekeny,

Could you do me the favor of resending as a unified diff? (diff -u)

Thanks!On Thu 17.Dec’09 at 23:24:11 +0100, BÁLINT Bekény wrote:

Hi Jesse,

I attached a patch for 3.8.6.
I’m not sure it is the best solution for this problem but it seems a working
one.
Hope I helped.

Best regards,

Bekeny

2009/12/17 Jesse Vincent jesse@bestpractical.com

On Thu 17.Dec'09 at 13:43:41 +0100, BALINT Bekeny wrote:
> Hi List,
>
>
> I have the following problem in 3.8.6 (sorry I can't try it on 3.8.7
now).
> If I have a header line in an email like this (or in Cc field):
>
> To: "'Foo Bar'" <foo.bar@foobar.hu>
> of
> To: 'Foo2 Bar2' <foo2.bar2@foobar2.hu>
>
> on the Update.html page (UpdateCc component) RT generates the code:
>
> [...]
>
> It seems that ' disturbs the generated JavaScript code.
> Is it a known issue?

I didn't know about it.

> Or a Bug?

It sure looks like a bug. Will you open a ticket? A patch would be much
appreciated if you or anyone else is up to it.

> Or a Feature?

It's definitely not one of these.

-Jesse

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAksqQg0ACgkQEi9d9xCOQEbYEACeN5Wphc16WbGraqy9ayXsPeVt
uHYAn1I2sS4AHEqkKNeK4uY+TZRwgjao
=N12W
-----END PGP SIGNATURE-----

*** share/html/Ticket/Elements/UpdateCc Mon Nov 23 23:11:39 2009
— local/html/Ticket/Elements/UpdateCc Thu Dec 17 23:14:00 2009


*** 55,61 ****
id=“UpdateCc-<%$addr%>”
name=“UpdateCc-<%$addr%>”
type=“checkbox”
! onClick=“checkboxToInput(‘UpdateCc’, ‘UpdateCc-<%$addr%>’,‘<%$txn_addresses{$addr}->format%>’ ); $(UpdateIgnoreAddressCheckboxes).value=1”
<% $ARGS{‘UpdateCc-’.$addr} ? ‘checked=“checked”’ : ‘’%> > <& /Elements/ShowUser, Address => $txn_addresses{$addr}&>
%}

--- 55,63 ---- id="UpdateCc-<%$addr%>" name="UpdateCc-<%$addr%>" type="checkbox" ! % my $clean_addr = $txn_addresses{$addr}->format; ! % $clean_addr =~ s/'//g; ! onClick="checkboxToInput('UpdateCc', 'UpdateCc-<%$addr%>','<%$clean_addr%>' ); $(UpdateIgnoreAddressCheckboxes).value=1" <% $ARGS{'UpdateCc-'.$addr} ? 'checked="checked"' : ''%> > <& /Elements/ShowUser, Address => $txn_addresses{$addr}&> %} *************** *** 66,72 **** id="UpdateBcc-<%$addr%>" name="UpdateBcc-<%$addr%>" type="checkbox" ! onClick="checkboxToInput('UpdateBcc', 'UpdateBcc-<%$addr%>','<%$txn_addresses{$addr}->format%>' ); $(UpdateIgnoreAddressCheckboxes).value=1" <% $ARGS{'UpdateBcc-'.$addr} ? 'checked="checked"' : ''%>> <& /Elements/ShowUser, Address => $txn_addresses{$addr}&> %} --- 68,76 ---- id="UpdateBcc-<%$addr%>" name="UpdateBcc-<%$addr%>" type="checkbox" ! % my $clean_addr = $txn_addresses{$addr}->format; ! % $clean_addr =~ s/'//g; ! onClick="checkboxToInput('UpdateBcc', 'UpdateBcc-<%$addr%>','<%$clean_addr%>' ); $(UpdateIgnoreAddressCheckboxes).value=1" <% $ARGS{'UpdateBcc-'.$addr} ? 'checked="checked"' : ''%>> <& /Elements/ShowUser, Address => $txn_addresses{$addr}&> %}

signature.asc (197 Bytes)

Of course.

Bekény2009/12/18 Jesse Vincent jesse@bestpractical.com

Bekeny,

Could you do me the favor of resending as a unified diff? (diff -u)

Thanks!

On Thu 17.Dec’09 at 23:24:11 +0100, BÁLINT Bekény wrote:

Hi Jesse,

I attached a patch for 3.8.6.
I’m not sure it is the best solution for this problem but it seems a
working
one.
Hope I helped.

Best regards,

Bekeny

2009/12/17 Jesse Vincent jesse@bestpractical.com

On Thu 17.Dec'09 at 13:43:41 +0100, BALINT Bekeny wrote:
> Hi List,
>
>
> I have the following problem in 3.8.6 (sorry I can't try it on

3.8.7

now).
> If I have a header line in an email like this (or in Cc field):
>
> To: "'Foo Bar'" <foo.bar@foobar.hu>
> of
> To: 'Foo2 Bar2' <foo2.bar2@foobar2.hu>
>
> on the Update.html page (UpdateCc component) RT generates the code:
>
> [...]
>
> It seems that ' disturbs the generated JavaScript code.
> Is it a known issue?

I didn't know about it.

> Or a Bug?

It sure looks like a bug. Will you open a ticket? A patch would be

much

appreciated if you or anyone else is up to it.

> Or a Feature?

It's definitely not one of these.

-Jesse

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAksqQg0ACgkQEi9d9xCOQEbYEACeN5Wphc16WbGraqy9ayXsPeVt
uHYAn1I2sS4AHEqkKNeK4uY+TZRwgjao
=N12W
-----END PGP SIGNATURE-----

*** share/html/Ticket/Elements/UpdateCc Mon Nov 23 23:11:39 2009
— local/html/Ticket/Elements/UpdateCc Thu Dec 17 23:14:00 2009


*** 55,61 ****
id=“UpdateCc-<%$addr%>”
name=“UpdateCc-<%$addr%>”
type=“checkbox”
! onClick=“checkboxToInput(‘UpdateCc’,
‘UpdateCc-<%$addr%>’,‘<%$txn_addresses{$addr}->format%>’ );
$(UpdateIgnoreAddressCheckboxes).value=1”
<% $ARGS{‘UpdateCc-’.$addr} ? ‘checked=“checked”’ : ‘’%> > <&
/Elements/ShowUser, Address => $txn_addresses{$addr}&>
%}

--- 55,63 ---- id="UpdateCc-<%$addr%>" name="UpdateCc-<%$addr%>" type="checkbox" ! % my $clean_addr = $txn_addresses{$addr}->format; ! % $clean_addr =~ s/'//g; ! onClick="checkboxToInput('UpdateCc',

‘UpdateCc-<%$addr%>’,‘<%$clean_addr%>’ );
$(UpdateIgnoreAddressCheckboxes).value=1"

  <% $ARGS{'UpdateCc-'.$addr} ? 'checked="checked"' : ''%> > <&

/Elements/ShowUser, Address => $txn_addresses{$addr}&>

%}

*************** *** 66,72 **** id="UpdateBcc-<%$addr%>" name="UpdateBcc-<%$addr%>" type="checkbox" ! onClick="checkboxToInput('UpdateBcc',

‘UpdateBcc-<%$addr%>’,‘<%$txn_addresses{$addr}->format%>’ );
$(UpdateIgnoreAddressCheckboxes).value=1"

      <% $ARGS{'UpdateBcc-'.$addr} ? 'checked="checked"' : ''%>>

<& /Elements/ShowUser, Address => $txn_addresses{$addr}&>
%}
— 68,76 ----
id=“UpdateBcc-<%$addr%>”
name=“UpdateBcc-<%$addr%>”
type=“checkbox”
! % my $clean_addr = $txn_addresses{$addr}->format;
! % $clean_addr =~ s/‘//g;
! onClick="checkboxToInput(‘UpdateBcc’,
‘UpdateBcc-<%$addr%>’,’<%$clean_addr%>’ );
$(UpdateIgnoreAddressCheckboxes).value=1"
<% $ARGS{‘UpdateBcc-’.$addr} ? ‘checked=“checked”’ : ‘’%>>
<& /Elements/ShowUser, Address => $txn_addresses{$addr}&>
%}

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAksr5l8ACgkQEi9d9xCOQEY0cQCgooq/46j4HCDwE5miUkMto6nv
DZgAoLU0rQvdttmcTwYHw8RRf5xvr1b+
=PnES
-----END PGP SIGNATURE-----

clean_address_in_RT_3_8_6.patch (1.29 KB)

Of course.


Bekény

> I have the following problem in 3.8.6 (sorry I can't try it on

3.8.7

now).
> If I have a header line in an email like this (or in Cc field):
>
> To: "'Foo Bar'" <foo.bar@foobar.hu>
> of
> To: 'Foo2 Bar2' <foo2.bar2@foobar2.hu>
>
> on the Update.html page (UpdateCc component) RT generates the code:
>
> [...]
>
> It seems that ' disturbs the generated JavaScript code.
> Is it a known issue?

Looking at the patch now that it’s a unified diff, I think we probably
want to be escaping the ’ rather than killing it. Does \’ work as a
replacement string?

-J

Of course.


Bekény

> I have the following problem in 3.8.6 (sorry I can't try it on

3.8.7

now).
> If I have a header line in an email like this (or in Cc field):
>
> To: "'Foo Bar'" <foo.bar@foobar.hu>
> of
> To: 'Foo2 Bar2' <foo2.bar2@foobar2.hu>
>
> on the Update.html page (UpdateCc component) RT generates the

code:

>
> [...]
>
> It seems that ' disturbs the generated JavaScript code.
> Is it a known issue?

Looking at the patch now that it’s a unified diff, I think we probably
want to be escaping the ’ rather than killing it. Does \’ work as a
replacement string?

Yes, it works.

Bekeny

Looking at the patch now that it's a unified diff, I think we probably
want to be escaping the ' rather than killing it.   Does \\' work as a
replacement string?

Yes, it works.


Bekeny

Thanks Bekény, applied to 3.8 as cf29b12.

Shawn