Problem/Motivation

Both \Drupal\Component\Utility\Html::getId and \Drupal\Component\Utility\Html::getClass are available as Twig filters (clean_id and clean_class respectively), but \Drupal\Component\Utility\Html::getUniqueId is not available.

Proposed resolution

Add a clean_unique_id Twig filter that calls back to \Drupal\Component\Utility\Html::getUniqueId.

Release notes snippet

To do.

Comments

alexdmccabe created an issue. See original summary.

alexdmccabe’s picture

Status: Active » Needs review
StatusFileSize
new836 bytes

Adding the patch, but I'm not familiar with front end / Twig testing process. I'm guessing this probably needs the needs-tests tag.

mherchel’s picture

+1

alexdmccabe’s picture

Issue tags: +fldc2020

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.

joycehutch’s picture

Testing #2 does not produce unique ids for the same menu block placed twice in the same region and page. The use case is for small screen/large screen; depending on screen size, one is hidden via CSS display: none;

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.

phjou’s picture

+1

mherchel’s picture

StatusFileSize
new796 bytes

Re-roll attached.

mherchel’s picture

Issue summary: View changes
StatusFileSize
new178.9 KB
neslee canil pinto’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new453.16 KB
new551.48 KB

@joycehutch tested adding 2 menu blocks inside a single region/page and it's generating unique ids for HTML. Please find the screenshots, this seems to be working fine. Moving it to RTBC.

jedihe’s picture

I can confirm #10 works.

There is, though, a caching problem that may cause duplicated IDs: Dynamic Page Cache will store unique IDs resolved for a given request, which \Drupal\Component\Utility\Html won't know about when rendering new markup via a #lazy_builder. I'm attaching a small patch + video showing how to replicate the error in a vanilla 9.3.x install, using the standard profile. Given this bug is not caused by this patch, IMO, it's fine for #10 to be RTBC.

I recently searched for a core issue to address this caching problem, but didn't yet find one. I think it's solvable by storing the IDs resolved for finalized markup stored in Dynamic Page Cache, then feeding those into \Drupal\Component\Utility\Html before resolving fragments via #lazy_builders.

Update: it looks like the caching problem I described is not exclusive to markup stored in Dynamic Page Cache cache bin, the same problem seems to (potentially) occur for markup stored in render cache bin. My best idea for a fix so far is:

  1. Get \Drupal\Component\Utility\Html to support restoring the state of resolved IDs (like it did up to 8.0.0-beta11?)
  2. Add a getter to \Drupal\Component\Utility\Html, to read all resolved IDs
  3. Update both render cache and Dynamic Page Cache to store the resolved HTML IDs present in the cached markup in the same cache record.
  4. When reading a record from the cache, restore the state of resolved IDs in \Drupal\Component\Utility\Html (8.0.0-beta11 had that capability)

Another option could be randomizing the generated HTML IDs (via a random suffix), which seems to be the chosen approach that led to 8.0.0-beta12 having the capabilities described above removed).

catch’s picture

catch’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs issue summary update

It would be good to have some justification in the issue summary for the increase in API surface - i.e. what is the use-case for using this within Twig?

Overall we've been trying to reduce the usage of Html::getUniqueId() across core for several years now (although still very far from zero), see #1305882: drupal_html_id() considered harmful; remove ajax_html_ids to use GET (not POST) AJAX requests and other issues.

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.

gonssal’s picture

@catch an example use-case is if you want to show system's Breadcrumbs block in more than one place, for example at the top and the bottom of your site. All themes in core have the system-breadcrumb id hard-coded on the template, so you get duplicated ids, which is invalid HTML. This also happens with local tasks for example.

You can of course use a preprocess function, but it's cumbersome when a simple Twig function would do it, specially if you need to override several templates.

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.

daniel korte’s picture

@catch another use case is a better DX for themers. Creating HTML ids outside of Drupal is a common practice and this would help to prevent collisions with those generated by Drupal.

skaught’s picture

+1.Access to this in a twig filter is very needed. am using this patch (on D9 project) with patch on comment 10.

works as expected. indeed: very useful for theming project.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new170 bytes

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

luke.leber’s picture

@catch:

Overall we've been trying to reduce the usage of Html::getUniqueId() across core for several years now

Unique ID's are a major accessibility need. Is there an alternative API mechanism that the community should be using here instead?

Until very recently, I don't think that I've ever worked on a theme that didn't render breadcrumbs at both the top and bottom of a page. It's been quite...cumbersome to work around this using Custom PHP on N clients. Given that we have a ~2 year grace period these days between disruptive changes within core, this is a moderately serious concern for a continuous upgrade path for folks that don't have a budget for dealing with breaking changes.

selvakumar-96’s picture

StatusFileSize
new796 bytes

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.

Ratan Priya’s picture

Status: Needs work » Needs review
StatusFileSize
new791 bytes
new791 bytes

Tried to re-roll patch #24 for 11.x-dev.

shashank5563’s picture

Status: Needs review » Reviewed & tested by the community

I have tested #24 patch on my local and found it is working as expected. So I am moving for RTBC.

  • lauriii committed 677faa7b on 11.x
    Issue #3115445 by mherchel, Ratan Priya, jedihe, alexdmccabe, selvakumar...

  • lauriii committed 4873ef5c on 10.1.x
    Issue #3115445 by mherchel, Ratan Priya, jedihe, alexdmccabe, selvakumar...
lauriii’s picture

Version: 11.x-dev » 10.1.x-dev
Status: Reviewed & tested by the community » Fixed

I definitely would have benefitted from this patch in the past because I would much prefer doing this in Twig than in preprocess.

Committed 677faa7 and pushed to 11.x. Also cherry-picked to 10.1.x. Thanks!

ressa’s picture

Nice improvement! I first tried to get it via 10.1.x, but it seemed like it wasn't available yet ... luckily it was ready in Drupal 11, so I could try it. This is pretty cool.

Update: @berdir helped me realize that Drupal 11 really is Drupal 10.2, and created #3363770: Clarify that current version 11 is 10.2.

Status: Fixed » Closed (fixed)

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