Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Follow-up to #2407361: Move usages of drupal_html_id() to Html::getUniqueId()
ViewPreviewForm contains dead code which was for tracking generated HTML IDs. Remove it:
// Reset the cache of IDs. Drupal rather aggressively prevents ID
// duplication but this causes it to remember IDs that are no longer even
// being used.
$seen_ids_init = &drupal_static('drupal_html_id:init');
$seen_ids_init = array();
Comment | File | Size | Author |
---|---|---|---|
#9 | viewpreviewform_should-2443847-9.patch | 786 bytes | cilefen |
#8 | viewpreviewform_should-2443847-8.patch | 959 bytes | cilefen |
#5 | viewpreviewform_should-2443847-5.patch | 827 bytes | cilefen |
#5 | interdiff-2-5.txt | 540 bytes | cilefen |
#2 | viewpreviewform_should-2443847-2.patch | 909 bytes | cilefen |
Comments
Comment #1
cilefen CreditAttribution: cilefen commentedComment #2
cilefen CreditAttribution: cilefen commentedI removed the inline comment also because the function doc for resetSeenIds is clear enough.
Comment #3
dawehnerWhy do we loose the documentation here?
Comment #4
cilefen CreditAttribution: cilefen commented@dawehner I said in #2 it seems redundant. Disagree? I don't feel strongly about it either way though.
Comment #5
cilefen CreditAttribution: cilefen commentedThis is with the comment.
Comment #6
dawehnerThank you for that!
Comment #7
alexpottThis is a bug because this code can't be working at this point. Any chance we can add a test for this?
Comment #8
cilefen CreditAttribution: cilefen commentedI think there is no need to reset the seen IDs here because the ID of preview form wrapper is always
views-preview-wrapper
. This class never tries to generate an ID.Comment #9
cilefen CreditAttribution: cilefen commentedComment #10
dawehnerAre you sure we also use no ID at all for CSS and what not?
Comment #11
cilefen CreditAttribution: cilefen commentedI am not totally sure. But it looks as though the Ajax wrapper IDs are hardcoded in this form so this serves no purpose. This static isn't set anywhere nor is
$seen_ids_init
even used.Comment #12
Wim LeersIsn't this simply dead, obsolete code? Grep HEAD for
drupal_html_id
, this is the only match. So I don't see why we'd need tests for that?Exactly. And on top of that, the
Html
class is only used to generate a unique ID if#id
isn't already set — see this inFormBuilder
:Comment #13
cilefen CreditAttribution: cilefen commentedComment #14
alexpottThis issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed e219cb9 and pushed to 8.0.x. Thanks!