Problem/Motivation

The form should not use overridden config, because this means the override will be saved to the database when submitting the form.

Steps to reproduce

Add this to settings.php

$config['simple_sitemap.settings']['base_url'] = 'https://' . $main_host;

Browse to /admin/config/search/simplesitemap/settings.
The overridden URL is displayed in the form field.

Proposed resolution

Use ConfigFactory::getEditable() to load the config.

Issue fork simple_sitemap-3359679

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

prudloff created an issue. See original summary.

prudloff opened merge request !66

prudloff’s picture

prudloff CreditAttribution: prudloff at Insite for Muséum national d'Histoire naturelle commented
Status: Active » Needs review
gbyte’s picture

gbyte CreditAttribution: gbyte as a volunteer and at gbyte commented
Version: 4.1.4 » 4.x-dev
Category: Bug report » Task
Status: Needs review » Postponed (maintainer needs more info)

Your reasoning makes sense but I wonder if you know of any other places (like core) known to do it your way? Also we'd have to scan the whole code base including submodules if this change is to be accepted.

prudloff’s picture

prudloff CreditAttribution: prudloff at Insite for Muséum national d'Histoire naturelle commented

It is explained here.

The core uses it for example in GDToolkit::buildConfigurationForm() or in ThemeExperimentalConfirmForm.

#2910353: Prevent saving config entities when configuration overrides are applied also talks about preventing saving the configuration when it contains overrides.

gbyte’s picture

gbyte CreditAttribution: gbyte as a volunteer and at gbyte commented
Status: Postponed (maintainer needs more info) » Needs work

This makes sense. I'm happy to merge this as soon as all forms are taken under account (including submodules). Feel free to grab it!

elber made their first commit to this issue’s fork.

elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
CreditAttribution: elber at CI&T commented
Status: Needs work » Needs review

Hi I just rebased please revise.

  • gbyte committed 8b4c37a2 on 4.x authored by prudloff 
    Issue #3359679 by prudloff, elber, gbyte: SettingsForm should not use...
gbyte’s picture

gbyte CreditAttribution: gbyte as a volunteer and at gbyte commented
Status: Needs review » Fixed

Commited to dev. Thank you!

Status: Fixed » Closed (fixed)

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