WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 6 months ago

#51256 closed defect (bug) (fixed)

Duplicate HTML IDs for checkboxes in Plugins list table

Reported by: SergeyBiryukov Owned by: SergeyBiryukov
Milestone: 5.6 Priority: normal
Severity: normal Version:
Component: Plugins Keywords: has-patch commit
Focuses: administration Cc:

Description

Background: #49916

If you have two plugins with the same name, but in different directories, the corresponding checkboxes on Plugins screen have duplicate ID attributes:

[DOM] Found 2 elements with non-unique id #checkbox_a55f1e83ab4fee1b42772d08e9971a9c: (More info: https://goo.gl/9p2vKq) 
<input type="checkbox" name="checked[]" value="debug-bar/debug-bar.php" id="checkbox_a55f1e83ab4fee1b42772d08e9971a9c">
<input type="checkbox" name="checked[]" value="debug-bar-old/debug-bar.php" id="checkbox_a55f1e83ab4fee1b42772d08e9971a9c">

Something like [48374] should be implemented for these checkboxes too.

Attachments (5)

51256.patch (1.5 KB) - added by kishanjasani 8 months ago.
Add a validation to restrict same ID generation
51256.2.patch (821 bytes) - added by mukesh27 8 months ago.
Patch with different solution.
51256.3.diff (806 bytes) - added by Hareesh Pillai 7 months ago.
51256.includes-update-core.diff (1.3 KB) - added by sabernhardt 6 months ago.
option with both the plugins page and the update-core page
51256.diff (1.2 KB) - added by helen 6 months ago.

Download all attachments as: .zip

Change History (19)

@kishanjasani
8 months ago

Add a validation to restrict same ID generation

#1 @kishanjasani
8 months ago

  • Keywords has-patch added

#2 @SergeyBiryukov
8 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#3 @mukesh27
8 months ago

I have tested patch but it is not working for me it show me Array in id attribute.

#4 @kishanjasani
8 months ago

@mukesh27 It's working for me. I have tested it on ubuntu 18.04. can you please attach screenshot which output array you are getting in ID attribute? So, I can go further to improve my code.

Last edited 8 months ago by kishanjasani (previous) (diff)

#5 @mukesh27
8 months ago

@kishanjasani I have checked patch and it working fine now for me.

I have another approach for the solution. We can use $plugin_id_attr variable because it will use unique plugin id.

@mukesh27
8 months ago

Patch with different solution.

#6 @kishanjasani
8 months ago

Yes, we can use that too. If the id value variable name doesn't matter.

#7 @Hareesh Pillai
7 months ago

  • Keywords needs-refresh added

Failing to apply the patch cleanly. Marking for refresh.

#8 @Hareesh Pillai
7 months ago

  • Keywords needs-refresh removed

Sharing the test results -

Before patch:

<input type="checkbox" name="checked[]" value="debug-bar/debug-bar.php" id="checkbox_7723712c996358d091e67d12c55e0c79">
<input type="checkbox" name="checked[]" value="debug-bar-old/debug-bar.php" id="checkbox_7723712c996358d091e67d12c55e0c79">

After patch:

<input type="checkbox" name="checked[]" value="debug-bar/debug-bar.php" id="checkbox_3181a37392941a47d4bf52bcd57f8c31">
<input type="checkbox" name="checked[]" value="debug-bar-old/debug-bar.php" id="checkbox_197a0c40d6f1064ea9fb5f9fa449cc19">

#9 @kishanjasani
7 months ago

@hareesh-pillai same patch is already added by @mukesh27.

#10 @sabernhardt
6 months ago

Thanks for the patch and the refresh. 51256.3.diff works for me on the Plugins screen.

<label class="screen-reader-text" for="checkbox_3775c0de7d0f58c2defbc4e193e54702">Select Widget Options</label>
<input type="checkbox" name="checked[]" value="widget-options/plugin.php" id="checkbox_3775c0de7d0f58c2defbc4e193e54702" />

<label class="screen-reader-text" for="checkbox_5e234580d0aeec7d9f5df882c7233442">Select Widget Options</label>
<input type="checkbox" name="checked[]" value="widget-options2/plugin.php" id="checkbox_5e234580d0aeec7d9f5df882c7233442" />

We probably should do the same for the update-core page, replacing $plugin_data->Name with $plugin_file. Would opening a separate ticket for that be better than adding it here?

@sabernhardt
6 months ago

option with both the plugins page and the update-core page

#11 @sabernhardt
6 months ago

In case both belong together, I added the update-core page change in 51256.includes-update-core.diff. If it's better separate, I'll open a ticket for that part later.

@helen
6 months ago

#12 @helen
6 months ago

I think my preference here would be to also use $plugin_file in the plugins list table (it does appear to be available and usable) so that it's the same between locations if, for whatever WordPress reason, somebody wants to consistently target that checkbox. See 51256.diff, which works in my testing.

#13 @helen
6 months ago

  • Keywords commit added

#14 @SergeyBiryukov
6 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 49631:

Plugins: Make sure the HTML ID attributes for plugin checkboxes are unique.

Follow-up to [48374].

Props helen, sabernhardt, kishanjasani, mukesh27, hareesh-pillai.
Fixes #51256.

Note: See TracTickets for help on using tickets.