Closed (fixed)
Project:
Drupal core
Version:
10.1.x-dev
Component:
render system
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
23 Feb 2020 at 19:15 UTC
Updated:
14 Jun 2023 at 08:54 UTC
Jump to comment: Most recent, Most recent file

Comments
Comment #2
alexdmccabeAdding the patch, but I'm not familiar with front end / Twig testing process. I'm guessing this probably needs the needs-tests tag.
Comment #3
mherchel+1
Comment #4
alexdmccabeComment #6
joycehutch commentedTesting #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;
Comment #9
phjou+1
Comment #10
mherchelRe-roll attached.
Comment #11
mherchelComment #12
neslee canil pinto@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.
Comment #13
jedihe commentedI 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:
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).
Comment #14
catch@jedihe, see #1852090: Cached render elements can have duplicate HTML IDs for the caching issue.
Comment #15
catchIt 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.
Comment #18
gonssal@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-breadcrumbid 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
preprocessfunction, but it's cumbersome when a simple Twig function would do it, specially if you need to override several templates.Comment #20
daniel korte@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.
Comment #21
skaught+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.
Comment #22
needs-review-queue-bot commentedThe 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.
Comment #23
luke.leber@catch:
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.
Comment #24
selvakumar-96 commentedComment #26
Ratan Priya commentedTried to re-roll patch #24 for 11.x-dev.
Comment #27
shashank5563 commentedI have tested #24 patch on my local and found it is working as expected. So I am moving for RTBC.
Comment #30
lauriiiI 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!
Comment #31
ressaNice 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.