WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 4 weeks ago

#49916 closed enhancement (fixed)

Add id attributes to the plugin activate/deactivate buttons

Reported by: shooper Owned by: SergeyBiryukov
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: Plugins Keywords: good-first-bug has-patch
Focuses: Cc:

Description

I'm working on a project that includes browser-based testing of a plugin, including it's installation and activation on the build server.

When trying to activate my plugin, the "Activate" link has no id attribute. Currently you have to drill down from the data-slug in the <tr> element to find the link.

It would be nice to add this with "activate-<slug>" or "deactivate-<slug>" so that you can easily find that link using tools like Selenium.

Attachments (3)

49916.diff (5.5 KB) - added by roytanck 4 months ago.
Adds id attributes the the activate, deactivate, resume and delete links on the plugin screen in wp-admin.
Plugins.png (51.9 KB) - added by mukesh27 4 months ago.
New and Old plugin
49916.2.diff (5.0 KB) - added by SergeyBiryukov 4 months ago.

Download all attachments as: .zip

Change History (15)

#1 @SergeyBiryukov
4 months ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to 5.5

@roytanck
4 months ago

Adds id attributes the the activate, deactivate, resume and delete links on the plugin screen in wp-admin.

#2 @roytanck
4 months ago

  • Keywords has-patch added; needs-patch removed

#3 follow-up: @mukesh27
4 months ago

  • Keywords needs-refresh added

@roytanck, I tested the patch 49916.diff and found one bug.

<a href="%s" aria-label="%s" id-"deactivate-%s">%s</a>

Replace to

<a href="%s" aria-label="%s" id="deactivate-%s">%s</a>

Plz correct it and upload a new patch.

#4 @SergeyBiryukov
4 months ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 47588:

Plugins: Add HTML ID attributes to plugin action links for easier targeting in browser-based testing.

Props roytanck, shooper.
Fixes #49916.

#5 in reply to: ↑ 3 @SergeyBiryukov
4 months ago

  • Keywords needs-refresh removed

Replying to mukesh27:

I tested the patch 49916.diff and found one bug.

<a href="%s" aria-label="%s" id-"deactivate-%s">%s</a>

Thanks for testing! Just noting this is already corrected in [47588].

Sorry for missing props, haven't refreshed the tab before committing :)

@mukesh27
4 months ago

New and Old plugin

#6 follow-up: @mukesh27
4 months ago

@SergeyBiryukov For the above situation, the new changes will show the same/duplicate id for Activate and Delete button.

Do we need to reopen the ticket?

#7 in reply to: ↑ 6 @SergeyBiryukov
4 months ago

  • Keywords needs-patch added; has-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to mukesh27:

For the above situation, the new changes will show the same/duplicate id for Activate and Delete button.

Good catch, thanks!

#8 follow-up: @roytanck
4 months ago

Any ideas how to fix the duplicate id issue? For the reporter's use case, I can imagine adding the plugin's version number to the id attribute may be a good solution? Another option could be to make is a class, but with browser-based testing that would still activate both plugins.

#9 in reply to: ↑ 8 ; follow-up: @SergeyBiryukov
4 months ago

  • Keywords has-patch added; needs-patch removed

Replying to roytanck:

Any ideas how to fix the duplicate id issue?

Using the plugin's directory and file name would give us unique IDs:

activate-akismet/akismet.php
activate-akismet-old/akismet.php

These are technically valid in HTML5, however, as explained in this Stack Overflow answer, the dot might need to be escaped when using these IDs.

For the purpose of fixing this edge case, I think it would be enough to save the IDs previously displayed on the page in a static array, and then add an incrementing suffix if we find a duplicate, same as wp_unique_post_slug() does:

activate-akismet
activate-akismet-2

Something like 49916.2.diff.

#10 @mukesh27
4 months ago

Hi there!

Patch 49916.2.diff working fine for me with unique ID values.

#11 in reply to: ↑ 9 @roytanck
4 months ago

Replying to SergeyBiryukov:

Using the plugin's directory and file name would give us unique IDs:

activate-akismet/akismet.php
activate-akismet-old/akismet.php

These are technically valid in HTML5, however, as explained in this Stack Overflow answer, the dot might need to be escaped when using these IDs.

I'd probably go for a data attribute instead of an id if we're going to add the path. Like is done on the <tr> element.

For the purpose of fixing this edge case, I think it would be enough to save the IDs previously displayed on the page in a static array, and then add an incrementing suffix if we find a duplicate, same as wp_unique_post_slug() does:

activate-akismet
activate-akismet-2

Something like 49916.2.diff.

I've tested your patch, and it works perfectly for me. Elegant solution, although at the expense of a little additional code. I prefer this solution over using the path.

#12 @SergeyBiryukov
4 weeks ago

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

In 48374:

Plugins: Make sure the HTML ID attributes in plugin action links are unique.

Follow-up to [47588].

Props mukesh27, roytanck, SergeyBiryukov.
Fixes #49916.

Note: See TracTickets for help on using tickets.