[wp-trac] [WordPress Trac] #46964: ID attribute value are used multiple times in "Custom Field" form

[WordPress Trac]

#46964: ID attribute value are used multiple times in "Custom Field" form
-------------------------------------------------+-------------------------
 Reporter:  jankimoradiya                        |       Owner:  audrasjb
     Type:  defect (bug)                         |      Status:  reopened
 Priority:  normal                               |   Milestone:  5.4
Component:  Editor                               |     Version:  5.0
 Severity:  normal                               |  Resolution:
 Keywords:  has-screenshots has-patch early      |     Focuses:  ui,
  commit needs-dev-note                          |  accessibility
-------------------------------------------------+-------------------------
Changes (by SergeyBiryukov):

 * status:  closed => reopened
 * resolution:  fixed =>


Comment:

 * If we're changing `#postbox-container-2` to a class, then other
 `#postbox-container-*` instances should be changed too for consistency,
 otherwise [source:trunk/src/wp-
 admin/includes/dashboard.php?rev=47222&marks=245,248,251,254#L230 this
 just looks weird] for no particular reason without any context:
 {{{
 <div id="postbox-container-1" class="postbox-container">
 <div class="postbox-container-2 postbox-container">
 <div id="postbox-container-3" class="postbox-container">
 <div id="postbox-container-4" class="postbox-container">
 }}}

 * With [https://wpdirectory.net/search/01E0NRARJT6F63TPEZ03QA1HN4 5200+
 plugins using #poststuff or #postbox-container-2], changing these IDs to
 classes seems like a large back-compat issue. Not all of them would be
 affected, but this is still a huge number.

 If the goal is to avoid duplicate IDs in `the_block_editor_meta_boxes()`,
 then let's do just that.

 For some context, in the Classic Editor, `#poststuff` was used to wrap
 almost the entire edit form, not just the meta boxes. Ideally, just moving
 `#poststuff` outside of the loop would work for our purposes:
 {{{
 <div id="poststuff" class="sidebar-open">
         <div id="postbox-container-2" class="postbox-container">
                 <?php foreach ( $locations as $location ) : ?>
                         <form class="metabox-location-<?php echo esc_attr(
 $location ); ?>" onsubmit="return false;">
                                 <?php
                                 do_meta_boxes(
                                         $current_screen,
                                         $location,
                                         $post
                                 );
                                 ?>
                         </form>
                 <?php endforeach; ?>
         </div>
 </div>
 }}}

 Unfortunately, the block editor moves these boxes around in the DOM, and
 while the location forms are visible, `#poststuff` ends up in a hidden
 area, and this change alone is not enough for this approach to work as
 expected.

 Related notes:
  * The `.sidebar-open` class is apparently not used anywhere and can be
 removed.
  * The `#postbox-container-2` ID doesn't provide any difference in
 `the_block_editor_meta_boxes()` and can also be removed (as mentioned in
 [https://github.com/WordPress/gutenberg/pull/4697 PR4697]). Its usage here
 is also not quite correct, historically it's only used for `normal` and
 `advanced` meta boxes, not for `side` meta boxes, which used `postbox-
 container-1` instead as a wrapper.

 I think the best solution here would be to reconsider
 [https://github.com/WordPress/gutenberg/pull/4697 PR4697] on the Gutenberg
 side, which tried to address the issue, but gave `#poststuff` too broad a
 scope, resulting in
 [https://github.com/WordPress/gutenberg/pull/4697#issuecomment-364117969
 CSS bleed] and eventual revert in
 [https://github.com/WordPress/gutenberg/pull/4971 PR4971]. If `#poststuff`
 is just moved outside of the loop and `#postbox-container-2` is removed
 from `the_block_editor_meta_boxes()`, that would resolve duplicate IDs
 without any back-compat issues.

 If we're fine with a potential breaking change for 5200+ plugins, I
 suggest we go the distance and convert the remaining `#postbox-
 container-*` IDs to classes to avoid issues like this in the future. See
 [attachment:"46964.2.diff"].

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/46964#comment:30>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform