Skip to content
Snippets Groups Projects

Added the condition taking care of the infinte loop

Closes #2640088

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
316 316
317 317 // If the default values for this element have not been loaded yet, populate
318 318 // them.
319 if (isset($elements['#type']) && empty($elements['#defaults_loaded'])) {
319 if ((isset($elements['#type']) && !isset($elements['#theme'])) && empty($elements['#defaults_loaded'])) {
  • Can you explain here why isset($elements['#theme']) was chosen as the condition, and why it's a solution that won't compromise other functionality? This may require a bit of research - and if researching leads you to a better option that's fine.

    Once the choice is researched and explained, note that there is a is a comment above this condition explaining the condition that you've now changed - this comment needs to be updated to describe the full condition (which now includes either isset($elements['#theme']) or potentially a better alternative you've found in your research)

  • I have added the comment according to my best understanding.

    1. Is there never a need to load defaults for elements with #theme set?
    2. This mentions The condition checks if the element has a type set (indicating it's a form element) This is not correct - the #type property is for any render element, not just form. Grepping the codebase for #type shows many examples of this
    3. This is crucial because elements // with a specific theme set might have been customized elsewhere, and loading // defaults for them could interfere with these customizations. I'm not sure this is the case - this line has been in the codebase for almost 10 years and such customizations have worked fine. This is presented as a widespread issue regarding customization of render elements, but seems very specific to form labels and how they are handled in form.inc (which isn't really customizing as it is a foundational part of rendering a form).

    Keep taking advantage of xdebug and the documentation within the codebase to try to understand what exactly is happening here. What you discover might lead you to a better solution. Doing this properly and knowledgeably is a higher priority than just getting a solution through as this is not a bug that is actively causing problems, but reintroducing a feature that would be nice to have but apparently is not critical as it hasn't existed since Drupal 8.0.

  • Please register or sign in to reply
  • utkarsh_33 added 1 commit

    added 1 commit

    Compare with previous version

  • utkarsh_33 resolved all threads

    resolved all threads

  • utkarsh_33 changed target branch from 11.0.x to 11.x

    changed target branch from 11.0.x to 11.x

  • utkarsh_33 changed target branch from 11.x to 11.0.x

    changed target branch from 11.x to 11.0.x

  • utkarsh_33 added 220 commits

    added 220 commits

    • 0dff1241...7dcb3aca - 210 earlier commits
    • e81e6f86 - Issue #3156439 by alexpott, catch, bonsaipuppy, mkalkbrenner, longwave, xjm:...
    • 902eab09 - Issue #3318988 by andypost, longwave, quietone: Finish deprecating status code...
    • 900d64b9 - Issue #3456133 by narendraR, VinmayiSwamy: Add validation constraints to search.page.*
    • a0466c55 - Issue #3457766 by narendraR, smustgrave: Add validation constraints to language.entity.*
    • 9ddbc2c7 - Issue #3449259 by narendraR, alexpott, catch, Wim Leers: Add validation...
    • 11c4be53 - Issue #3458922 by quietone, alexpott, longwave: Fix index test in...
    • 9490eb3e - Issue #3458751 by alexpott: Drupal 10.3.x regression running JS tests using...
    • 392ee750 - Added the condition taking care of the infinte loop
    • ea1ed77b - Fixed tests
    • 67a34859 - Added comments

    Compare with previous version

  • utkarsh_33 changed target branch from 11.0.x to 11.x

    changed target branch from 11.0.x to 11.x

  • Please register or sign in to reply
    Loading