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

[bug] Fix - Email copy to own address doesn't work since 2.5.21 #3771

Closed
wants to merge 2 commits into from
Closed

[bug] Fix - Email copy to own address doesn't work since 2.5.21 #3771

wants to merge 2 commits into from

Conversation

Kubik-Rubik
Copy link
Member

Related to
[bug] [33848] Fix fatal error when sending contact form (#3763)

How to test

  • Use Joomla! version 2.5.22 and create a contact - set a link to the contact with a form in the menu
  • Fill out the form with some sample data (but correct email address), activate the checkbox "Send copy to yourself" and click on send
  • You will never receive a copy of this email because the value of this checkbox input field is empty
  • Add patch and try it again

Related to bug - [bug] [33848] Fix fatal error when sending contact form
@Kubik-Rubik
Copy link
Member Author

@zero-24
Copy link
Contributor

zero-24 commented Jun 13, 2014

@Kubik-Rubik i can confirm the issue and the fix 👍

@Bakual
Copy link
Contributor

Bakual commented Jun 13, 2014

I'm thinking that we should just revert the actual check in the controller and fix it there. It's not necessary a given that any contact form uses JForm to create that checkbox.
HTML Checkboxes are a funny thing. The value of the checkbox actually doesn't matter at all. You have to check if they are submitted, not if they contain a value. The value is only relevant if you have multiple checkboxes with the same name.
On a sidenote, that's actually done wrong in 3.x as well...

@Kubik-Rubik
Copy link
Member Author

@Bakual

I think that a value parameter should be used in a checkbox field and so we can use this solution to fix the bug!

Yes, we could also only optimize the if statement. Instead of checking !empty($data['contact_email_copy']) we could just check whether this request variable is set at all isset($data['contact_email_copy']). If the checkbox is selected, then the request variable is set with an empty value. With the current check this can never be true without providing a value. In Joomla! 3.x the value is set to 1, so the initial fix does work there.

@roland-d
Copy link
Contributor

Just before I test, should I test the value 1 or the proposed controller change? Confused.

@Bakual
Copy link
Contributor

Bakual commented Jun 14, 2014

I would use isset personally. Mainly because we don't really know where the form is coming from. It may well be an override (or even a module) which doesn't use JForm and thus would still be broken.
I don't want to risk another broken thing :)
The XML change can of course be kept as well. It's always better :)

…equest variable is empty if the input field has no value parameter even though the checkbox is activated
@Kubik-Rubik
Copy link
Member Author

@Bakual Correct, updated the PR.
@roland-d Now you can test the whole PR. :-) Thanks!

@roland-d
Copy link
Contributor

@test: After applying the patch, I do get a copy of the message sent via the contact form.

JC tracker moved to RTC.

@Bakual Bakual added the RTC label Jun 16, 2014
Bakual pushed a commit that referenced this pull request Jun 23, 2014
@Bakual
Copy link
Contributor

Bakual commented Jun 23, 2014

Merged with 09e8d05

@Bakual Bakual closed this Jun 23, 2014
@Kubik-Rubik Kubik-Rubik deleted the 2.5.x-fix-com_contact-email-copy branch June 20, 2015 18:10
@zero-24 zero-24 removed the RTC This Pull Request is Ready To Commit label Oct 14, 2015
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.

None yet

5 participants