Make WordPress Core

Opened 7 weeks ago

Last modified 2 weeks ago

#60663 assigned defect (bug)

Plugin Cards: Links with `.button-disabled` are still clickable

Reported by: costdev's profile costdev Owned by: costdev's profile costdev
Milestone: 6.6 Priority: normal
Severity: normal Version:
Component: Plugins Keywords: has-patch
Focuses: accessibility Cc:

Description

Props to @swissspidy for noticing this while testing Plugin Dependencies.

Currently, links with a href attribute and the .button-disabled class are still clickable.

We should prevent the default click handling for elements with the .button-disabled class.

While this appears to have been the case in earlier WordPress versions, it may be more apparent in the increased AJAX-ified experience added by the Plugin Dependencies feature.

Milestoning for 6.5 for awareness, though I welcome thoughts on whether this is something that needs to be handled in 6.5, or should wait until 6.6.

Testing Instructions

These steps define how to reproduce the issue, and indicate the expected behavior.

Steps to Reproduce

  1. Navigate to Plugins > Add New Plugin.
  2. Click "Install Now".
  3. Once installed, click "Activate".
  4. 🐞 Once activated, click on the disabled "Active" button.

Expected Results

When testing a patch to validate it works as expected:

  • ✅ The link does not work.

When reproducing a bug:

  • ❌ The link still works despite the .button-disabled class being present.

Change History (9)

This ticket was mentioned in PR #6207 on WordPress/wordpress-develop by @costdev.


7 weeks ago
#1

Previously, links with a href attribute and the .button-disabled class were still clickable.

This prevents the default handling for elements with the .button-disabled class.

#2 @swissspidy
7 weeks ago

  • Focuses accessibility added

@swissspidy commented on PR #6207:


7 weeks ago
#3

I was thinking more of adding the disabled HTML attribute instead, because that's what we do on a fresh page load already:

https://github.com/WordPress/wordpress-develop/blob/186eeafb6888431fdf445b2abf2036f0606edcd5/src/wp-admin/includes/plugin-install.php#L1007-L1011

But turns out there it is an actual <button> and here it's still a regular link.

So I suppose this makes sense 👍

Looks like the button-disabled class is also only used for shiny updates, so this shouldn't affect anything else either.

Requesting review from accessibility folks just in case.

#4 @huzaifaalmesbah
7 weeks ago

After applying PR 6207, I tried clicking the button, but it didn't work ✅

#5 @audrasjb
7 weeks ago

  • Milestone changed from 6.5 to 6.6

As the proposed PR is still being discussed, let's move this ticket to milestone 6.6.

@afercia commented on PR #6207:


6 weeks ago
#6

On the "regression", I would agree that it's a regression from an accessibility perspective due to the markup of the link/button. Once that's improved, I think the removal of the automatic redirect is an enhancement that will benefit all users to be able to install and activate multiple plugins without leaving their current context.

Well, I'd argue the Activate link that stays clickable is a regression so it's not just accessibility, it's general usability.
I'm not opposed to 'AJAX-ifing' things. However, given this page is basically becoming sort of 'single page app', many other things should have been addressed. I do realize my consideration doesn't help solving the issue and I agree that right now we can only introduce a temporary fix. However, I'd still consider the changes to the install plugin page an incomplete development that will need to be addressed soon in the future.

@costdev commented on PR #6207:


6 weeks ago
#7

The still clickable activate link is absolutely a bug. The accessibility regression I was referring to was the use of an a element rather than a button element, and the limited interaction options that provides (click/enter vs click/enter/space) while enabled. That, plus the incorrect semantics of using a here, and disabling it rather than use a button. While a temporary fix is in order for now, we should definitely make this correct and consistent for Plugin Dependencies, the Plugins/Themes screens (modals included), and Corewide as soon as possible.

@afercia commented on PR #6207:


2 weeks ago
#9

plus the incorrect semantics of using a her

I'm not sure I clarified why many admin pages use an <a> element that behaves like a button.

Some new contributors may not know that, historically, WordPress supported the scenario where users may have JavaScript support disabled in their browsers. That's not a so rare scenario. Some companies or government bodies or other organizations may still disable JavaScript support as a security policy for their employees browseers. For yeaers, supporting 'JS off' has been a concern for any development in WordPress.

Many of these links in WordPress still work when JS is off. They just use a standard GET request e.g. update.php?action=upgrade-plugin&plugin={plugin filename here}&_wpnonce={nonce here}. Instead, when JS is on, they use an AJAX request counterpart e.g. update-plugin.

That's the way 'JS off' has been historically supported. I'm not saying that was a perfect approach as it creates a clear problem with links that behave like buttons but still semantically are links. This has been discussed many times bu tthere's no real way to fully addess this issue.

Personally I'm not opposed to stop supporting the 'JS off' scenario. That would allow to change these links and just use buttons. However, I think that is a decision that would need a broader discussion and can't be made in this PR.

ella225 commented on PR #6207:


2 weeks ago
#10

The presence of a still clickable 'activate' link despite being disabled is a bug that needs fixing. Additionally, the accessibility regression, using an <a> instead of a <button> element, must be addressed promptly for consistent functionality and improved accessibility across the platform."

Edit: Spam URLs removed before comment deletion.

Note: See TracTickets for help on using tickets.