This regression was introduced via #140783: A select list without #default_value always passes form validation.

If you install a clean new copy of D7 after that patch, the installer defaults the country selection to 'Afghanistan' (the first in the list). It used to default to '- None -'.

Not sure if this is major, beta blocker, or what. But, webchick wanted a new issue for this so I'm getting it started...

CommentFileSizeAuthor
#10 934110-10.installer_no_default_country.patch1.84 KBdww
#5 934110-default-country.patch1.31 KBdamien tournoud
#3 934110-default-country.patch759 bytesdamien tournoud
#1 934110-1.installer_no_default_country.patch705 bytesdww

Comments

dww’s picture

dww
we/he/they
commented
Status: Active » Needs review
StatusFileSize
new 934110-1.installer_no_default_country.patch705 bytes

No idea if this is the best solution here, but this is what the admin UI does at admin/config/regional/settings. WFM on a clean install. If we wanted to exactly replicate the pre #140783 UI we could easily use t('- None -') here, instead of being consistent with admin/config/regional/settings...

damien tournoud’s picture

damien tournoud commented

The best way would be to add #empty_value => '' to this element (assuming I understand the other issue correctly).

damien tournoud’s picture

damien tournoud commented
StatusFileSize
new 934110-default-country.patch759 bytes

Indeed.

dww’s picture

dww
we/he/they
commented

Shouldn't we do the same in system_regional_settings() then?

damien tournoud’s picture

damien tournoud commented
StatusFileSize
new 934110-default-country.patch1.31 KB

Indeed.

markabur’s picture

markabur commented

Looks good. Tested #5, default country is back to "- None -".

sun’s picture

sun
Primary language German
Location Karlsruhe
commented 
+++ includes/install.core.inc
@@ -1759,6 +1759,7 @@ function _install_configure_form($form, &$form_state, &$install_state) {
     '#title' => t('Default country'),
+    '#empty_value' => '',
     '#default_value' => variable_get('site_default_country', NULL),
     '#options' => $countries,
     '#description' => st('Select the default country for the site.'),

Yes, that's correct, for both.

BTW, can someone explain me WTF this Default country is? It's only ever set in the installer, only get in System regional settings, and the description does not explain what it "could" be good for.

Powered by Dreditor.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Reviewed & tested by the community

That said, that's a different issue.

dww’s picture

dww
we/he/they
commented
Status: Reviewed & tested by the community » Needs work

If we're fixing system_regional_settings() we need to do it right. One sec, I'll re-roll.

dww’s picture

dww
we/he/they
commented
Status: Needs work » Needs review
StatusFileSize
new 934110-10.installer_no_default_country.patch1.84 KB

Like so...

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Needs review » Reviewed & tested by the community

RTBC if it comes back green.

juan_g’s picture

juan_g commented
Status: Reviewed & tested by the community » Needs review

sun wrote:

BTW, can someone explain me WTF this Default country is? It's only ever set in the installer, only get in System regional settings, and the description does not explain what it "could" be good for.

It seems to have been introduced for D7 core by #333156: Add ability to configure site default country. They said:

This patch is just a first step towards supporting country specific date and number formats. It doesn't provide any per-country functionality (yet), but the idea is that in the future, both core and contrib modules can add country-specific features without each module defining their own method to configure the default country for the site.

This country setting (for time zones, languages, date formats, etc.) is also provided for D6 by the Site Country module, used by Country code. Both seem to be experimental modules.

juan_g’s picture

juan_g commented

It's green now...

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Fixed

Yay!! Committed to HEAD!

(Sorry, Damien, I was too eager to commit this and missed your name on the commit message. I'll add you twice next time.)

Status: Fixed » Closed (fixed)
Issue tags: -Usability

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue tags: +Usability

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.