Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core/issues/322 - fix JS error on contribution page, completing form… #12652

Merged
merged 1 commit into from
Aug 17, 2018

Conversation

davejenx
Copy link
Contributor

…on behalf of someone else, when profile includes checkbox field.

See https://lab.civicrm.org/dev/core/issues/322 .

Overview

On a contribution page, when "completing this form on behalf of someone else", a JavaScript syntax error occurs when the page includes a profile with a checkbox field and the contact has this field checked. This PR fixes that.

Replicated on demo site. Steps to replicate:

  1. Create a custom field, Test Checkbox: Alphanumeric, CheckBox with 1 option: label "I agree", value 1.
  2. Create a profile including this field.
  3. Include this profile on a contribution page.
  4. Edit Contact A and check the "I agree" checkbox.
  5. On the contribution page, click "Not demo test, or want to do this for a different person?"
  6. Choose Contact A.

Before

JavaScript error: "Syntax error, unrecognized expression: [name=custom_13[1]]".
Form does not populate with Contact A's data.

After

Form populates with Contact A's data.

Technical Details

Simple JS fix: ensure name attribute is properly quoted.

…orm on behalf of someone else, when profile includes checkbox field.
@civibot
Copy link

civibot bot commented Aug 13, 2018

(Standard links)

@eileenmcnaughton
Copy link
Contributor

@davejenx this looks like a regression from the security release (5.3.2) are you able to confirm?

@davejenx
Copy link
Contributor Author

@eileenmcnaughton I have replicated the issue on 4.7.31, so evidently not a regression from the security release.

@eileenmcnaughton
Copy link
Contributor

thanks @davejenx

Copy link
Contributor

@jitendrapurohit jitendrapurohit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have quickly replicated with the steps mentioned in the description and can confirm that this fixes the js issue displayed in the console.

Thanks @davejenx

@davejenx
Copy link
Contributor Author

Thanks, @jitendrapurohit !

@seamuslee001
Copy link
Contributor

Merging as per review from @jitendrapurohit

@seamuslee001 seamuslee001 merged commit 050aceb into civicrm:master Aug 17, 2018
@eileenmcnaughton
Copy link
Contributor

@jitendrapurohit @davejenx @seamuslee001 I just closed the issue against 5.6 - keep an eye on this as we are missing that step a bit

@davejenx
Copy link
Contributor Author

Thanks @seamuslee001 & @eileenmcnaughton !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants