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

[3.x] send emails to dotless domains #28576

Merged
merged 2 commits into from May 3, 2020
Merged

Conversation

brianteeman
Copy link
Contributor

@brianteeman brianteeman commented Apr 4, 2020

Now that we are using phpmailer 6'+ this code block is not needed as it is now the default. However it does not support dotless domains which joomla supports so we still need to set a validator.

It was an error to change from using auto to php for email address validation as the php validation does not support dotless domains which joomla supports so we have to use html5 as the validator.

this is a backport of #28574

technically this pr is a b/c break as it allows dotless domains and it has not allowed allowed dotless domains since #24726 was merged but its much more of a bug fix as we should have allowed dotless domains and that pr broke it

Testing Instructions

see #28574

Now that we are using phpmailer 6'+ this code block is not needed as it is now the default. However it does not support dotless domains which joomla supports so we still need to set a validator.

this is a backport of joomla#28574

technically this pr is a b/c break as it allows dotless domains and it has not allowed allowed dotless domains since joomla#24726 was merged but its much more of a bug fix as we should have allowed dotless domains and that pr broke it
@gerryfrancis
Copy link

Now that we are using phpmailer 6'+

I am not sure if I understand this well, but AFAIK, 3.9.17 will use PHPMailer 5.2.28, right?

@brianteeman
Copy link
Contributor Author

shoot I didnt check. i just saw that 5.2 was not supported by phpmailer even for security and incorrectly assumed we had therefore updated.

However that misassumption doesnt change the need for this pr or invalidate the changes. I will update the original post

@ChristineWk
Copy link

I have tested this item ✅ successfully on 09960ab


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28576.

@infograf768
Copy link
Member

infograf768 commented Apr 14, 2020

Could this, as well as #28574 have an impact on utf8 domains?

@brianteeman
Copy link
Contributor Author

Test it and see

@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 09960ab


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28576.

@Quy
Copy link
Contributor

Quy commented Apr 16, 2020

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/28576.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 16, 2020
@brianteeman
Copy link
Contributor Author

@zero-24 any reason this hasnt been merged?

@HLeithner
Copy link
Member

Could this, as well as #28574 have an impact on utf8 domains?

I'm waiting for this test.

@brianteeman
Copy link
Contributor Author

there is no reason for it to fail. the html5 validate really just checks for something@something or something@something.tld unlike the php version which doesnt test for the first

using utf8 or punycode doesnt introduce any . or @ as that would obviously break all email so there is no reason it could fail

@HLeithner
Copy link
Member

You requested the test not me. So I assume it's working. thanks

@HLeithner HLeithner merged commit 1b23d0d into joomla:staging May 3, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 3, 2020
@HLeithner HLeithner added this to the Joomla! 3.9.19 milestone May 3, 2020
@brianteeman brianteeman deleted the phpmailer3 branch May 3, 2020 15:59
@brianteeman
Copy link
Contributor Author

thanks

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

8 participants