Problem/Motivation

Regular elements constructed in buildForm() render with an ID. Also, the container gets the same ID. So, the page has two elements with the same ID.

Example:

<?php
$form['system_compact_link'] = array(
  '#type' => 'system_compact_link',
}
?>

We were able to work around this in the linked issue by adding:

<?php
'#id' => FALSE,
?>

Proposed resolution

Do not add an ID attribute to an element that already has a container with the same ID.

Remaining tasks

First of all, should Form building allow non-form types? or, is this change actually needed?
Write a test.
Decide on how to fix it. If going with the current solution, fix remaining test fail.

Original report by @lokapujya

Follow-up to #1833932: Convert theme_system_compact_link() into a #type link

CommentFileSizeAuthor
#26 interdiff-2347209-24-26.txt1.69 KBheykarthikwithu
#26 2347209-26.patch2.18 KBheykarthikwithu
#24 2347209-24.patch3.29 KBlokapujya
#22 2347209-22-test-only.patch3.29 KBlokapujya
#20 2347209-18b-test-only.patch2.17 KBlokapujya
#18 2347209-18-test-only.patch2.28 KBlokapujya
#16 render_regular_element-2347209-16.patch687 byteslauriii
#11 2347209-11.patch687 byteslokapujya
#8 interdiff-2347209-7.txt712 byteslokapujya
#8 2347209-7.patch687 byteslokapujya
#6 2347209-6.patch647 byteslauriii
#4 remove-id-from-element-only.patch587 byteslokapujya
#1 regular-render-elements.patch1.27 KBlokapujya
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya commented
Status: Active » Needs review
FileSize
regular-render-elements.patch1.27 KB

This code demos where the IDs get added. The patch gets rid of both IDs. Not sure if we should change the render code or use ['#markup'] = drupal_render().

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya commented
Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 1: regular-render-elements.patch, failed testing.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya commented
Status: Needs work » Needs review
FileSize
remove-id-from-element-only.patch587 bytes

Status: Needs review » Needs work

The last submitted patch, 4: remove-id-from-element-only.patch, failed testing.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii commented
Status: Needs work » Needs review
FileSize
2347209-6.patch647 bytes

Status: Needs review » Needs work

The last submitted patch, 6: 2347209-6.patch, failed testing.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
2347209-7.patch687 bytes
interdiff-2347209-7.txt712 bytes

attempt to fix 1st test fail, by not removing id for containers.

Status: Needs review » Needs work

The last submitted patch, 8: 2347209-7.patch, failed testing.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Related issues: +#2226323: CompositeFormElementTrait causes identical HTML ids for element and its wrapper

Adding a loosely related issue.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya commented
Status: Needs work » Needs review
FileSize
2347209-11.patch687 bytes
star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Category: Support request » Task
Status: Needs review » Needs work
Issue tags: +Needs issue summary update, +Needs tests

So I guess this is a bug/task? Please update the issue summary/category as appropriate. This line of code is a bit complex so should probably have a code comment to explain the logic and we'd want to add tests as well.

Thanks @lokapujya!

The last submitted patch, 11: 2347209-11.patch, failed testing.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya commented
Issue summary: View changes
lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya commented
Issue summary: View changes
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Druid commented
Version: 8.0.x-dev » 8.2.x-dev
Status: Needs work » Needs review
FileSize
render_regular_element-2347209-16.patch687 bytes

Re uploading the previous patch to see results on new test bot

Status: Needs review » Needs work

The last submitted patch, 16: render_regular_element-2347209-16.patch, failed testing.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya at Babson College commented
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
2347209-18-test-only.patch2.28 KB

Status: Needs review » Needs work

The last submitted patch, 18: 2347209-18-test-only.patch, failed testing.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya at Babson College commented
Status: Needs work » Needs review
FileSize
2347209-18b-test-only.patch2.17 KB

Status: Needs review » Needs work

The last submitted patch, 20: 2347209-18b-test-only.patch, failed testing.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya at Babson College commented
Status: Needs work » Needs review
FileSize
2347209-22-test-only.patch3.29 KB

File missing from patch, Don't know why that happened.

Status: Needs review » Needs work

The last submitted patch, 22: 2347209-22-test-only.patch, failed testing.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya at Babson College commented
Title: Render regular element in a form. » Render regular element in a form should not duplicate container ID.
Category: Task » Bug report
Status: Needs work » Needs review
FileSize
2347209-24.patch3.29 KB

Still needs a comment for line mentioned in #12:

+++ b/core/includes/theme.inc
@@ -1124,7 +1124,9 @@ function template_preprocess_container(&$variables) {
+      if (isset($element['#type']) && isset($element['#parents']) && (($element['#type'] == 'container') || !in_array($element['#type'], $element['#parents']))) {

Something like this, but it doesn't sound right to me:
// If the element is a container or if the wrapper is the not the same type as the element then add an ID.

Status: Needs review » Needs work

The last submitted patch, 24: 2347209-24.patch, failed testing.

heykarthikwithu’s picture

heykarthikwithu
Primary language English
Location Bengaluru 🌍
CreditAttribution: heykarthikwithu as a volunteer and at Valuebound commented
Status: Needs work » Needs review
FileSize
2347209-26.patch2.18 KB
interdiff-2347209-24-26.txt1.69 KB

Status: Needs review » Needs work

The last submitted patch, 26: 2347209-26.patch, failed testing.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
CreditAttribution: lokapujya at Babson College commented 
+++ b/core/modules/system/src/Tests/Form/FormTest.php
@@ -757,8 +757,8 @@
--- b/core/modules/system/tests/modules/form_test/src/Form/FormTestRegularElementForm.php
+++ /dev/null

Why was this deleted?

Also, any clue to why the test is still failing?

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

luke.stewart’s picture

luke.stewart CreditAttribution: luke.stewart commented
Status: Needs work » Closed (cannot reproduce)
Issue tags: +Needs steps to reproduce, +Bug Smash Initiative

I've just attempted to replicate this on 10.2.4

I created a custom module using drush generate simple:form and installed

In the form I created the following insight the generated buildForm method

    $form['acontainer'] = [
	    '#type' => 'container',
	    '#title' => $this->t('Container'),
    ];

    $form['container']['atextfield'] = [
	    '#type' => 'textarea',
	    '#title' => 'one',
	    '#required' => TRUE,
    ];

When I inspect the elements on the page rendered I see a control with an id 'edit-atextfield' which has a wrapper div with id 'edit-acontainer'

If this doesn't demonstrate the problem please include some clear steps to reproduce.