Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.There's the base url of a system we need to make sure exists. Things like localhost make absolutely no sense.
Drupal lacks of a feature to know if the base url is set explicitly or if it was autocreated. If this is not configured explicitly and sending is via cron or drush, urls might also be broken.
We could add a new configuration directive and check for that..
Also supporting a 6.x-2.x backport due to significant quality improvement.
Comments
Comment #1
BerdirWondering if we need our own setting.. It's already possible to set it in settings.php, are there are situations where it makes sense to only specify it for simplenews? Maybe when you have multiple domains pointing to your site...
What about having multiple language specific domains which you use for language recognition? Then the links need to use the correct domain depending on the language of the user? Do we already cover this?
I think what makes sense is adding a check in the cron implementation to see if it's default, localhost, *.local or anything without atleast a single dot.
Comment #2
miro_dietikerMultiple domains: Not covered yet explicitly. However, possibly also not covered cleanly with setting the base_url manually in the settings.php.
Since we switch language while sending to the newsletter language, we have here bigger problems currently that needs additional consideration.
Checking locale/locale.module and includes/locale.inc you can see that negotiation is being done with locale_language_from_url() or locale_language_url_fallback().
I'm not perfectly sure but i suspect this code is only executed once. At least we need here a sending static cache clear.
Only focussing on that language detection is possibly wrong. There might be other modules that modify the URL detection and hook in hook_init like domain module.
Possibly we should group recipients by language and cover sending per-language in mutiple bootstrap requests (per language).
#996392: Multilanguage support
I consider the current multilanguage behavior here at least incomplete if not wrong.
Note that independent of the original configuration in settings.php, the base_url $conf gets updated / created by core code. So the base_url is always present in the result while it still can be nonsense (like "default"). Filtering out non-dotted notation might be enough here.
Comment #3
miro_dietikerAdd test candidate tag. Need to test for a per-language-domain setup.
Extended interaction with e.g. domain module would need even more tests. Dunno if needed.
Comment #4
BerdirRe: per-domain/path language set-up.
The relevant code for us is in locale_url_outbound_alter(), which seems to call locale_language_url_rewrite_url(), which relies on $options['language] or global $language_url (not sure where that is set).
Comment #5
BerdirFor the record, I don't think testing a domain per-language set-up is possible, but a subpath should work and be pretty much the same thing. As long as the language altering functions are called and with the correct language, we're good..
Comment #6
miro_dietikerI think we should add this test to make multilingual system work cleanly before release.
Related to
#1374926: Test and improve active language handling when building the newsletter
Comment #7
BerdirLanguage handling is fixed and tested over in #1374926: Test and improve active language handling when building the newsletter.
So I think there isn't much left to be done here. The base_url path check, but I'm actually not 100% sure that we need this. Many developers will want to test it on localhost or something like that, so it should be possible to disable it, maybe even disabled by default.. and then it's rather useless because it almost certainly won't be enabled when you had needed it.
Comment #8
miro_dietikerI see no reason to allow localhost mails being sent out by default.
Developers can learn that they need to change a setting. However i would output this setting with a warning in the requirements...
Outputting a watchdog/output and aborting the execution seems fine to me. For drush this should go to console.
There might be a warning like "set variable simplenews_cron_host_nocheck to TRUE to disable host validy checks"...
Related: simplenews_linkchecker with its checks before send.
Comment #9
miro_dietikerPushing...