Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#51256 closed defect (bug) (fixed)

Duplicate HTML IDs for checkboxes in Plugins list table

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


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: 
<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 4 years ago.
Add a validation to restrict same ID generation
51256.2.patch (821 bytes) - added by mukesh27 4 years ago.
Patch with different solution.
51256.3.diff (806 bytes) - added by Hareesh Pillai 4 years ago.
51256.includes-update-core.diff (1.3 KB) - added by sabernhardt 4 years ago.
option with both the plugins page and the update-core page
51256.diff (1.2 KB) - added by helen 4 years ago.

Download all attachments as: .zip

Change History (19)

4 years ago

Add a validation to restrict same ID generation

#1 @kishanjasani
4 years ago

  • Keywords has-patch added

#2 @SergeyBiryukov
4 years ago

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

#3 @mukesh27
4 years ago

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

#4 @kishanjasani
4 years ago

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

Version 0, edited 4 years ago by kishanjasani (next)

#5 @mukesh27
4 years 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.

4 years ago

Patch with different solution.

#6 @kishanjasani
4 years ago

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

#7 @Hareesh Pillai
4 years ago

  • Keywords needs-refresh added

Failing to apply the patch cleanly. Marking for refresh.

#8 @Hareesh Pillai
4 years 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
4 years ago

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

#10 @sabernhardt
4 years 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?

4 years ago

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

#11 @sabernhardt
4 years 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.

4 years ago

#12 @helen
4 years 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
4 years ago

  • Keywords commit added

#14 @SergeyBiryukov
4 years 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.