Problem/Motivation

Steps to reproduce this issue:

  1. Add a file field to the 'page' node-type, say 'field_file'.
  2. Go to /node/add/page.
  3. Inspect the file widget markup. You'll find this piece of markup:
    <div id="edit-field-file-0-upload" class="js-form-managed-file form-managed-file">
      <input data-drupal-selector="edit-field-file-0-upload" type="file" id="edit-field-file-0-upload" name="files[field_file_0]" size="22" class="js-form-file form-file">
      ...
    </div>
    

Note that the file input field and the div wrapper are sharing the same ID: edit-field-file-0-upload.

Proposed resolution

  • Add a new form element property #label_for which allows to associate the element labe with a different field by setting this property the ID of the desired field.
  • Manually generate an ID for the file input so that we can instruct the form element label to point to the file input element.
  • Add a regression test.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#92 2497909-92.interdiff.txt3.41 KBclaudiu.cristea
#92 2497909-92.patch7.29 KBclaudiu.cristea
#86 2497909-86.interdiff.txt1.58 KBclaudiu.cristea
#86 2497909-86.patch8.29 KBclaudiu.cristea
#84 2497909-84.interdiff.txt2.96 KBclaudiu.cristea
#84 2497909-84.patch8.92 KBclaudiu.cristea
#82 2497909-82.patch6.27 KBkim.pepper
#80 duplicate_html_ids_are-2497909-80.patch6.22 KBjibran
#68 interdiff.txt1.31 KBpfrenssen
#68 2497909-68-test_only.patch911 bytespfrenssen
#68 2497909-68.patch6.22 KBpfrenssen
#66 interdiff.txt764 bytespfrenssen
#66 2497909-66-test_only.patch870 bytespfrenssen
#66 2497909-66.patch6.25 KBpfrenssen
#62 interdiff-2497909-60-62.txt1.18 KByogeshmpawar
#62 duplicate_html_ids_are-2497909-62.patch6.03 KByogeshmpawar
#60 interdiff.txt1.47 KBpfrenssen
#60 2497909-60-test_only.patch695 bytespfrenssen
#60 2497909-60.patch6.08 KBpfrenssen
#59 2497909-59.patch5.25 KBpfrenssen
#44 interdiff.txt445 bytescilefen
#44 duplicate_html_ids_are-2497909-44.patch2.61 KBcilefen
#37 file-remove_duplicate_id-2497909-8-37.patch2.48 KBAndrew.Mikhailov
#33 file-remove_duplicate_id-2497909-8-33.patch3.17 KBAndrew.Mikhailov
#32 file-remove_duplicate_id-2497909-8-32.patch3.16 KBAndrew.Mikhailov
#24 file-remove_duplicate_id-2497909-8-24.patch603 bytesAndrew.Mikhailov
#18 file-remove_duplicate_id-2497909-7-18.patch511 bytesAndrew.Mikhailov
#16 file-remove_duplicate_id-2497909-7-16.patch542 bytesAndrew.Mikhailov
#13 file-remove_duplicate_id-2497909-7-13.patch504 bytesAndrew.Mikhailov
#10 file-remove_duplicate_id-2497909-7-9.patch580 bytesAndrew.Mikhailov
#8 file-remove_duplicate_id-2497909-7-7.patch580 bytesAndrew.Mikhailov
#5 file-remove_duplicate_id-2497909-5-7.patch580 bytesAndrew.Mikhailov
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cilefen’s picture

I cannot reproduce this. I added multiple file fields to content type in D7 and D8 and the issue does not occur. Does this happen when coding using the API? If so, please post the code you wrote that produces duplicate IDs.

Jeremy JOCHEMSKI’s picture

Hello,

Thanks for the answer. The issue concernes drupal v7.37.

Do you add an id to file fields for reproducing the issue (using '#id') ?

I use webform module which put id on file fields, but it calls the core
theme function which produce the html result and duplicate the id.

It's not by coding the api but I think it concernes the core function algorithm or my understanding :
https://api.drupal.org/api/drupal/modules%21file%21file.module/function/...

On /modules/file/file.module
Line 687-690 :
It instanciate an array.
If there is an id into the elements, it puts the same id into the array.

  $attributes = array();
  if (isset($element['#id'])) {
    $attributes['id'] = $element['#id'];
  }

Line 698 :
It creates a div with an id (so the same id of the file element)

  $output .= '<div' . drupal_attributes($attributes) . '>';

Line 699 :
It creates the render of the element with the call of function drupal_attributes
so with the same element id.

  $output .= drupal_render_children($element);

So, the function theme_file_managed_file set two similar id, one into the div elem, one into the input type=["file"] elem.

I hope that my explanation is usefull.

Best regards,

cilefen’s picture

So this is a webform module issue? Could it be a duplicate of #2059215: Replace use of IDs to classes on Webform wrappers to avoid duplicate IDs?

DanChadwick’s picture

@cilefen - Core file fields don't use theme_file_managed_file, which is why you aren't seeing the bug. I believe if you programmatically create the managed_file field you will be able to reproduce the issue. Core field use a theme_file_widget or theme_file_widget_multiple. See modules/file/file.field.inc.

EDIT: I'm the webform maintainer. Webform isn't creating the duplicate ID, core is via FAPI and this theme function.

Andrew.Mikhailov’s picture

Issue tags: +file
FileSize
580 bytes

Hello guys!
I have a problem with duplicate id on a project.
When I try to upload file I have JS-error.
http://dl1.joxi.net/drive/0005/3729/343697/150828/6a7508433a.png
Please apply my patch for your module.

Best regards.

Andrew.Mikhailov’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 5: file-remove_duplicate_id-2497909-5-7.patch, failed testing.

Andrew.Mikhailov’s picture

Status: Needs work » Needs review
FileSize
580 bytes

Status: Needs review » Needs work

The last submitted patch, 8: file-remove_duplicate_id-2497909-7-7.patch, failed testing.

Andrew.Mikhailov’s picture

Version: 7.37 » 7.x-dev
FileSize
580 bytes

Sorry, I used wrong drupal version.

Andrew.Mikhailov’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 10: file-remove_duplicate_id-2497909-7-9.patch, failed testing.

Andrew.Mikhailov’s picture

Status: Needs work » Needs review
FileSize
504 bytes

One more patch...

cilefen’s picture

Status: Needs review » Needs work

What about drupal_html_id()?

Andrew.Mikhailov’s picture

Good idea!
Thank you!
I'm going push it today.

Andrew.Mikhailov’s picture

Status: Needs work » Needs review
FileSize
542 bytes

Patch with drupal_html_id()

Status: Needs review » Needs work

The last submitted patch, 16: file-remove_duplicate_id-2497909-7-16.patch, failed testing.

Andrew.Mikhailov’s picture

Status: Needs work » Needs review
FileSize
511 bytes

One more time...

Andrew.Mikhailov’s picture

Hello guys!
Please check my patch.

Best regards.

cilefen’s picture

It could probably use a test. Are we sure this isn't a problem in Drupal 8?

Andrew.Mikhailov’s picture

No, this bug appear in my project on D7.
I also set patch version 7.x-dev.

cilefen’s picture

Component: theme system » file system

Issues must opened and fixed in Drupal 8 first if they affect Drupal 8 according to the backport policy.

Andrew.Mikhailov’s picture

Ok, I'll check it in D8.

Andrew.Mikhailov’s picture

Issue tags: +Drupal 8.x, +patch

I've tested it in PHP 5.5 & MySQL 5.5 13,985 and everything is ok.
Please apply my patch.
Best regards.

cilefen’s picture

Title: Duplicate ID for file_managed_file field (w3c validator get error) » Duplicate HTML IDs are created for file_managed_file fields (w3c validator error)
Issue tags: -file, -Drupal 8.x, -patch

These tags are not necessary.

Andrew.Mikhailov’s picture

Okay, sorry.

cilefen’s picture

Status: Needs review » Needs work
Issue tags: +Needs backport to D7, +Needs tests

There is no absolute "right" or "wrong" tagging rule in this issue queue. In practice, useful tags are for queue management purposes, such as "Needs reroll", "needs backport to D7", or tags for sprints.

It will be worthwhile to add a regression test for this. If you are unfamiliar with tests in core I can offer guidance. To start, look in the src/Tests directory in the file module. We may be able to add to an existing test.

Andrew.Mikhailov’s picture

Assigned: Unassigned » Andrew.Mikhailov
Andrew.Mikhailov’s picture

Hello!
Could you provide guidance for regression tests?
I can't find it.
Thank you.

cilefen’s picture

FileFieldDisplayTest is a possible place for this. For example, at the end of FileFieldDisplayTest::testNodeDisplay(), you could perhaps add a second file field then check the raw output for duplicate IDs.

Tip: Execute this test locally on your development computer, the re-execute it as you modify it.

Andrew.Mikhailov’s picture

Hello!
I implemented test for this, also I remove some unused variables and change deprecated functions to D8 methods.
And after my test we have problem with "edit-FIELD_NAME-ajax-wrapper", I can find duplicated IDs, but cannot solve it now. (I set TODO for this)
So, please check my current test and please apply my patch.
Best regards.

Andrew.Mikhailov’s picture

Sorry, I've create patch via phpStorm. Sometimes it creates invalid patches :(
So, one more patch...

Andrew.Mikhailov’s picture

Status: Needs work » Needs review

The last submitted patch, 32: file-remove_duplicate_id-2497909-8-32.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 33: file-remove_duplicate_id-2497909-8-33.patch, failed testing.

Andrew.Mikhailov’s picture

Status: Needs work » Needs review
FileSize
2.48 KB

Corrected last patch...

Andrew.Mikhailov’s picture

This is very strange but test fails in another places...
for example http://joxi.ru/MAjGeyzueXM1re
I think this is related to new Drupal CI.
Please review my patch manually and send me feedback.

Best regards.

Status: Needs review » Needs work

The last submitted patch, 37: file-remove_duplicate_id-2497909-8-37.patch, failed testing.

Andrew.Mikhailov’s picture

Status: Needs work » Needs review

Please check last comment.

The last submitted patch, 32: file-remove_duplicate_id-2497909-8-32.patch, failed testing.

The last submitted patch, 33: file-remove_duplicate_id-2497909-8-33.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 37: file-remove_duplicate_id-2497909-8-37.patch, failed testing.

cilefen’s picture

Status: Needs work » Needs review
FileSize
2.61 KB
445 bytes

The test failures are real, for example, ContentTranslationSyncImageTest. It is because the Html class was not imported with a use statement.

Andrew.Mikhailov’s picture

Yes, I see( Sorry.

David_Rothstein’s picture

I'm not sure the theme layer is the right place to fix this? See discussion/analysis at #2594955: [D7] Duplicate HTML IDs are created for file_managed_file fields which identifies this as a deeper problem (should probably be marked as a duplicate of this one)... at least for Drupal 7 the patch there may be the right way to go. I haven't looked into Drupal 8 yet.

Note to self - whatever we do here for Drupal 7, need to test how this affects IMCE Filefield since the committed fix for the previous regression there did not match the patch posted in the issue (#2466475: Drupal core 7.36 broke IMCE for FileField Ajax upload previews) and therefore could break again if it depends on specific IDs.

Andrew.Mikhailov’s picture

What about my patch?
Is it correct or not?

cilefen’s picture

Dragan Eror’s picture

Assigned: Andrew.Mikhailov » Unassigned
Status: Needs review » Reviewed & tested by the community

Tested patch from #44 and it works like a charm! Thanks @Andrew.Mikhailov!

cilefen’s picture

Status: Reviewed & tested by the community » Needs work
  1. +++ b/core/modules/file/src/Tests/FileFieldDisplayTest.php
    @@ -102,6 +101,13 @@ function testNodeDisplay() {
    +    if ($ajax_wrapper_id = array_search("edit-{$field_name}-ajax-wrapper", $matches[1])) {
    +      // @TODO need to fix it for "edit-FIELD_NAME-ajax-wrapper"
    +      unset($matches[1][$ajax_wrapper_id]);
    

    See the standard for @todo comments.

  1. +++ b/core/modules/file/src/Tests/FileFieldDisplayTest.php
    @@ -49,7 +49,6 @@ function testNodeDisplay() {
     
    -    $test_file = $this->getTestFile('text');
         simpletest_generate_file('escaped-&-text', 64, 10, 'text');
    

    Is changing this in the issue scope? Even if not, it could be ok in this test.

  2. +++ b/core/modules/file/src/Tests/FileFieldDisplayTest.php
    @@ -93,7 +92,7 @@ function testNodeDisplay() {
         $name = 'files[' . $field_name . '_1][]';
    -    $edit[$name] = drupal_realpath($test_file->getFileUri());
    +    $edit[$name] = \Drupal::service('file_system')->realpath($test_file->getFileUri());
     
         // Uncheck the display checkboxes and go to the preview.
    

    Is changing this in the issue scope? Even if not, it could be ok in this test.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Manuel Garcia’s picture

Issue tags: +w3c markup validator
Manuel Garcia’s picture

Title: Duplicate HTML IDs are created for file_managed_file fields (w3c validator error) » Duplicate HTML IDs are created for file_managed_file fields

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

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should 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.

cilefen’s picture

Issue tags: -Needs backport to D7

I renamed the D7 issue #2594955: [D7] Duplicate HTML IDs are created for file_managed_file fields because the backport policy allows it to exist. But we could think about merging these based on the reasoning in #46.

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

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should 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.

pfrenssen’s picture

Assigned: Unassigned » pfrenssen

I agree that the solution from #2594955: [D7] Duplicate HTML IDs are created for file_managed_file fields looks better, it proposes a new #label_for property that allows to declare for which (sub) element the label is intended. This approach allows to solve this problem not only for the managed file field but for any complex field that might have a form element nested inside a wrapper.

Assigning to me, I need to fix this for a D8 project today, I'll have a few hours this afternoon to work on this.

pfrenssen’s picture

I started on this, and it appears that the #label_for functionality has already been implemented for D8, but the property is called #for:

function template_preprocess_form_element_label(&$variables) {
  // ...

  // A #for property of a dedicated #type 'label' element as precedence.
  if (!empty($element['#for'])) {
    $variables['attributes']['for'] = $element['#for'];
    // A custom #id allows the referenced form input element to refer back to
    // the label element; e.g., in the 'aria-labelledby' attribute.
    if (!empty($element['#id'])) {
      $variables['attributes']['id'] = $element['#id'];
    }
  }
  // Otherwise, point to the #id of the form input element.
  elseif (!empty($element['#id'])) {
    $variables['attributes']['for'] = $element['#id'];
  }

  // ...
}

This has been added in #1938926: Convert simpletest theme tables to table #type.

pfrenssen’s picture

I marked #2594955: [D7] Duplicate HTML IDs are created for file_managed_file fields as a duplicate of this issue since the suggested approach there is already partly implemented in D8. This means we can use the same fix for both versions and we don't need two separate issues for it since we can use the normal patch workflow of fixing it first for D8 and then backporting to D7.

Here is an initial forward port of the original D7 patch from #2594955: [D7] Duplicate HTML IDs are created for file_managed_file fields. This has not yet been tested properly. No interdiff since this is a completely new approach.

pfrenssen’s picture

Assigned: pfrenssen » Unassigned
Issue tags: -Needs tests
FileSize
6.08 KB
695 bytes
1.47 KB

Added a test and fixed a bug that was uncovered by the test.

If this gets committed, please add credit to @predy and @David_Rothstein who wrote the original D7 patch. I only ported it to D8.

Status: Needs review » Needs work

The last submitted patch, 60: 2497909-60-test_only.patch, failed testing.

yogeshmpawar’s picture

Updated patch for test failures & removed one coding standard issue

Status: Needs review » Needs work

The last submitted patch, 62: duplicate_html_ids_are-2497909-62.patch, failed testing.

pfrenssen’s picture

It appears there is a *second* instance of a duplicate HTML ID, the ajax wrapper that is added in FileWidget::processMultiple.

pfrenssen’s picture

Component: file system » file.module

The second instance of duplicate HTML IDs is handled in #2346893: Duplicate AJAX wrapper around a file field.

pfrenssen’s picture

Status: Needs work » Needs review
FileSize
6.25 KB
870 bytes
764 bytes
pfrenssen’s picture

pfrenssen’s picture

I cross-posted with @Yogesh Pawar, here is a version of the patch including the fixes from #62.

The last submitted patch, 66: 2497909-66-test_only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 68: 2497909-68-test_only.patch, failed testing.

pfrenssen’s picture

Status: Needs work » Needs review
yogeshmpawar’s picture

Any Update on this issue ?

cilefen’s picture

cilefen’s picture

Oh, #65.

joseph.olstad’s picture

Priority: Normal » Major

bumping the priority because a Major requires this to be fixed.

star-szr’s picture

Status: Needs review » Needs work

I don't have a lot of deep thoughts on this one right now but one thing jumped out at me:

+++ b/core/includes/form.inc
@@ -433,6 +436,7 @@ function template_preprocess_form_element(&$variables) {
+    '#label_for' => [],

It's weird to me that this defaults to array, an empty string seems more appropriate.

star-szr’s picture

Also, the IS could use some love and this probably needs a CR because of the #label_for addition.

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

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should 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.

jibran’s picture

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

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should 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.

kim.pepper’s picture

Version: 8.5.x-dev » 8.6.x-dev
Status: Needs work » Needs review
FileSize
6.27 KB

Re-roll for 8.6.x

Status: Needs review » Needs work

The last submitted patch, 82: 2497909-82.patch, failed testing. View results

claudiu.cristea’s picture

Test fix. I wasn't able to use the AssertContentTrait because I'm getting this error:

Declaration of Drupal\KernelTests\AssertContentTrait::getAllOptions() should be compatible with Drupal\Tests\BrowserTestBase::getAllOptions(Behat\Mink\Element\NodeElement $element)

I think the trait is only for kernel tests (?)

claudiu.cristea’s picture

claudiu.cristea’s picture

claudiu.cristea’s picture

Status: Needs review » Needs work

The last submitted patch, 86: 2497909-86.patch, failed testing. View results

claudiu.cristea’s picture

Status: Needs work » Needs review

Cannot understand. Retesting.

jibran’s picture

Status: Needs review » Reviewed & tested by the community

I have been using this patch for quite a while and it fixes the a11y issue. It seems like all the outstanding feedback has been addressed so RTBC.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/file/tests/src/Functional/FileFieldDisplayTest.php
@@ -220,4 +226,38 @@ public function testDescriptionDefaultFileFieldDisplay() {
+    $status = TRUE;
+    foreach ($this->xpath('//*[@id]') as $element) {
+      $id = $element->getAttribute('id');
+      if (isset($seen_ids[$id]) && !in_array($id, $ids_to_skip)) {
+        $this->fail(new FormattableMarkup('The HTML ID %id is unique.', ['%id' => $id]));
+        $status = FALSE;
+      }
+      $seen_ids[$id] = TRUE;
+    }
+    $this->assertTrue($status, $message);

If there are no IDs to be found then this will pass. I'm not that that is correct. I think we should check that $seen_ids is not empty.

claudiu.cristea’s picture

Status: Needs work » Needs review
FileSize
7.29 KB
3.41 KB

@alexpott and myself we had a chat on the remark from #91:

clau [2:04 PM] @alexpott on https://www.drupal.org/project/drupal/issues/2497909. If there are no IDs then there are no duplicate IDs
So, the test should pass

alexpott [2:05 PM] @clau hmmm well if there are no IDs then why would you be testing for duplicates?
@clau negative tests with no positive assertions are always risky

clau [2:06 PM] The caller knows that there are IDs
So, the test should fail with the same message if no IDs?

alexpott [2:06 PM] Imo this test is very fragile and subject to reporting success even when that’s not the case.

clau [2:07 PM] Or should show a dedicated messages for no IDs?

alexpott [2:07 PM] @clau I would add a new assert that checks $seen_ids and has it’s own message.

clau [2:07 PM] ok
I can do it earlier, by checking the xpath result

alexpott [2:09 PM] Also you’re not only checking field fields you are checking the entire page
“Verify the file fields don’t contain duplicate HTML IDs.” should be “Verify the page does not contain duplicate HTML IDs.”
And $message needs a good default not “”

clau [2:11 PM] sure, the method has been copied because i cannot use the AssertContentTrait which works only for Kernel tests

alexpott [2:12 PM] Maybe $this->assertTrue($status, $message ?: ‘The page does not contain duplicate IDs’);
Where maybe the page has the URL tested

clau [2:13 PM] > maybe the page has the URL tested
@alexpott You mean to inject the url in the message?

alexpott [2:14 PM]
Yep

I also cleaned the assert method as it has been initially copied from the WTB and contains parameters that we are not using here.

jibran’s picture

Status: Needs review » Reviewed & tested by the community

Back to RTBC as #91 has been addressed.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 20c250e and pushed to 8.7.x. Thanks!

As this is adding a new render property to form elements only committing to 8.7.x

alexpott’s picture

Adding review credit.

  • alexpott committed 20c250e on 8.7.x
    Issue #2497909 by Andrew.Mikhailov, pfrenssen, claudiu.cristea, cilefen...
jibran’s picture

As this is adding a new render property to form elements only committing to 8.7.x

Sorry, I don't want to sound like a broken record who is complaining about backporting all around the issue queue. I'm happy we fixed it at least in 8.7.x but here are my concerns:

  • This is a major bug which it makes every D8 site fail a11y(which has a file field on the form esp. webform).
  • The property added here is not breaking BC in any way.
  • We are not messing with frozen templates.
  • The HTML change here is actually making sure that markup is valid which was invalid before this.

Given the above info I suggest we should backport this.

As a side note, I published the change record. Once again I really appreciate that you committed this. Thank you!

claudiu.cristea’s picture

Status: Fixed » Patch (to be ported)

Yes, please, it doesn't hurt an existing site in any way. Could you reconsider the decision? Thank you

alexpott’s picture

@jibran I disagree - the problem is you never know what some one is doing in contrib and custom and they might have a form element which already has this property and adding it might break their code. Yes this is not API and is specifically not BUT it is helpful to only add this kind of change at specific times.

alexpott’s picture

Status: Patch (to be ported) » Fixed

Status: Fixed » Closed (fixed)

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

Pancho’s picture

Version: 8.6.x-dev » 8.7.x-dev
Issue tags: -Needs backport to D7

Only fixed in 8.7.x, so changing version.
D7 backport is being covered in #2594955: [D7] Duplicate HTML IDs are created for file_managed_file fields, so removing tag.