Cannot submit site information form with empty front page field

Hmm, this probably happens for site_403 and site_404 as well

Re-rolling for the current dev branch

The issue seems to exist in the current dev branch as well.

Re-uploading test only patch, resolving a few deprecations.

I have tested the last patch,it works fine .

Before Patch

After Patch

+++ b/core/modules/system/src/Form/SiteInformationForm.php
@@ -162,7 +162,7 @@ public function validateForm(array &$form, FormStateInterface $form_state) {
       // Set to default "user/login".
-      $form_state->setValueForElement($form['front_page']['site_frontpage'], '/user/login');
+      $form_state->setValueForElement($form['front_page']['site_frontpage'], '/user');

I don't understand why we're changing this when it is blank. If this is the fix then the comment // Set to default "user/login". needs updating.

I think we need to reassess this patch. How is this ever blank? Ah I see... $front_page = $site_config->get('page.front') != '/user/login' ? $this->aliasManager->getAliasByPath($site_config->get('page.front')) : ''; is odd code.

I think we should be using the getUrlIfValidWithoutAccessCheck() check and not check access on these links. It's not relevant.

Using getUrlIfValidWithoutAccessCheck() solves the reported issue

Should we open a follow up to better handle the default value?

We'll also need to update the error message

Works for me, removes the strange error message.

Needs reroll for 9.3.x though.

Re-rolled it.
Kindly review it.

+++ b/core/modules/system/tests/src/Functional/System/FrontPageTest.php
@@ -72,7 +72,7 @@ public function testDrupalFrontPage() {
-    $this->assertSession()->pageTextContains("Either the path '" . $edit['site_frontpage'] . "' is invalid or you do not have access to it.");
+    $this->assertText("The path '" . $edit['site_frontpage'] . "' is invalid.");

We now need to use $this->assertSession()->pageTextContains() here.

Fixed test failure errors as suggested in #31, thanks!

Thanks! RTBC if bot agrees.

Let's not add a whole new test to maintain especially when we have \Drupal\Tests\system\Functional\System\FrontPageTest::testDrupalFrontPage already that uses this form and tests this logic already.

We could add to the bottom of \Drupal\Tests\system\Functional\System\FrontPageTest::testDrupalFrontPage - this test describes itself as

* Tests front page functionality and administration.

    // Ensure the user can revert the front page to the default of 'user/login'
    // even when they don't have access.
    $this->assertFalse(\Drupal::service('path.validator')->isValid('user/login'));
    $this->drupalGet('admin/config/system/site-information');
    $edit = ['site_frontpage' => ''];
    $this->submitForm($edit, 'Save configuration');
    $this->assertSession()->pageTextContains('The configuration options have been saved.');
    $this->drupalGet('');
    // We will we redirected to the user's account page.
    $this->assertSession()->addressEquals('user/2');
    $this->assertSession()->pageTextNotContains('On front page.');
    $this->drupalLogout();
    $this->assertSession()->addressEquals('user/login');
    $this->assertSession()->pageTextContains('On front page.');

Hi , I have modified #32 as per #34 suggestions, adding the interdict and patch for the same.

Thanks!

We need to explicitly set site_frontpage to an empty string for this test, we don't need to set the other settings here, and I think we should add a comment to explain the case we are testing.

Thanks @longwave for pointing out , I have modified the tests and added a description.

Please review.

LGTM.

The patch does not apply to 9.4.x and hasn't for a few months.

Rerolled the patch, thanks!

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Reroll looks good and issue is still addressed.

The issue summary is a statement of the problem. There is no indication of the proposed resolution.

I then went to apply the patch. I see that is against 9.4, which is in security support. The patch applied to 10.1.x so I tested it using the steps in the issue summary.

With the patch, it is now possible to save the basic sites settings page with an empty field for the default front page. I then looked at the system.site configuration.

$ ddev drush cget system.site
_core:
  default_config_hash: VDJxTZtQR21qB4lvOq8zszJZLvLKrSPQpdn2E3T71Ww
langcode: en
uuid: 76eacedf-7cf4-4707-86c7-c478fe9ef9f6
name: dev2-web
mail: v@example.com
slogan: ''
page:
  403: ''
  404: ''
  front: /user/login
admin_compact_mode: false
weight_select_max: 100
default_langcode: en

The front page path has not changed and we have the UI showing incorrect information. I then used the UI to change the the front page configuration to an empty string. I confirmed the change with drush front: ''. On navigating to the front page, I get a 404.

I don't know if that is the intended behavior. But having the UI and the stored configuration out of sync is wrong.

I am adding a tag for an issue summary update and setting back to needs work.

Re-Roll the Patch For 10.1 by #41.

#46 was unnecessary as patch 41 still applied to D10 hiding patch.

Also please include an interdiff with all patches

I forgot that I commented on this issue before. I propose simplifying this setting and making the field required over in #2671174: Make default front page setting required and remove special case of /user/login - this would mean this issue is no longer required.

As per #48 this is no longer required