Fix disabled publish/update button still clickable

[glingener]

Description

Recently the disabled attribute for the publish/update button was replaced by aria-disabled to avoid focus loss.
fc03492

The problem with aria-disabled is that it won't block the functionality of the button - saving is still possible, using disabled will block the functionality as expected.

In addition aria-disabled on a button is agains W3C standards:

Only use the aria-disabled attribute for elements that are not allowed to have a disabled attribute in HTML5

https://www.w3.org/TR/html-aria/#attr-disabled

How has this been tested?

Open a post and click "Update" without any changes. The post does not save.

Types of changes

Replaced aria-disabled with disabled.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.

Fixes #11809.

[logicalphase]

Is there any other benefit to having the save post button disabled? I'm not clear as to why the button condition needs to be disabled at all excepting a save in progress, but perhaps I'm missing the intended value otherwise. Thanks.

[glingener]

@HyperPress for example you want to have some required fields and use wp.data.dispatch( 'core/editor' ).lockPostSaving( 'required-fields-not-filled' ); to block the save button until all required fields are filled.

Currently with the aria-disabled the whole save lock control feature is worthless. #10649

[youknowriad]

I think the reason we prefer 'aria-disabled' is to avoid focus loss when the status of the button changes. So instead we do 'aria-disabled' and return early on the onClick handler. Maybe we're missing the early return here.

Is this right @afercia ?

[noisysocks]

I am not Andrea but yes that's right. We should be using aria-disabled and returning early from the onClick handler.

[noisysocks]

From #12073:

From an accessibility perspective, using aria-disabled="true" and "noop'ing" an UI control is equivalent to disabling it with a disable attribute, as long as it is perceived as disabled by all users, i.e. also visually. The relevant difference is that a control with aria-disabled="true" is still focusable. Removing focusability from disabled elements can offer users both advantages and disadvantages. A must read recommendation can be found in the ARIA authoring practices:

5.7 Focusability of disabled controls
https://www.w3.org/TR/wai-aria-practices-1.1/#kbd_disabled_controls

Although those recommendations apply to ARIA widgets, the Gutenberg top bar can be considered a "toolbar", where the ability to discover the functionalities within the toolbar is a primary concern. I'd recommend to adopt a consistent convention for the focusability of these controls and always use aria-disabled="true" / "noop". The Block navigation menu should not open its panel when there are no blocks.

[afercia]

Late reply but yes: when there's the need to avoid a focus loss (as when a focused button gets disabled while it's focused), it's possible to use aria-disabled and noop it. At that point, the only difference is that the button is still focusable. This can offer both advantages and disadvantages, to be evaluated on a case by case basis. For more details:

5.7 Focusability of disabled controls
https://www.w3.org/TR/wai-aria-practices-1.1/#kbd_disabled_controls

[logicalphase]

I wonder then if the main problem is in the sequence/timing as currently aria-disabled in this case does not prevent actions on the button. Investigating further. Thank you all for your feedback and clarifications.

[adamsilverstein]

currently aria-disabled in this case does not prevent actions on the button

Exactly! The only/main thing we need to do in the PR is prevent the click handlers when the button is disabled.

[adamsilverstein]

oops, thanks for fixing that milestoning @youknowriad - i was confused about how the milestones are being used.

[youknowriad]

No worries so we basically put WordPress * milestones for issues and 4.* for PRs.

[adamsilverstein]

@CGlingener I created an alternate solution that exists early from the onClick event handler if the button is disabled, can you test and verify this resolves your issue: #12885

[afercia]

Note: I've created an issue on the W3C "ARIA in HTML" GitHub to consider to clarify the current wording for aria-disabled. See w3c/html-aria#122

Worth also considering "ARIA in HTML" is still a working draft. Instead, the relevant spec to take into consideration is ARIA 1.1 and, for the future, ARIA 1.2:
https://www.w3.org/TR/wai-aria-1.1/#aria-disabled
https://www.w3.org/TR/wai-aria-1.2/#aria-disabled

[youknowriad]

Thanks for your work here, this is superseded by #12885