Treat aria-disabled buttons as disabled in Capybara

What does this MR do and why?

Teaches Capybara's disabled filter about aria-disabled, so click_button and click_on wait for a Pajamas button to become available instead of clicking one that will ignore them.

This replaces the opt-in aria_disabled filter added in !249594 (merged), which fixed one spec and required every other call site to opt in. That filter and its single call site are removed here.

The problem

GlButton keeps an unavailable button focusable: it renders aria-disabled="true" and computedListeners deletes the click listener, rather than setting the native disabled attribute.

isDisabledOrLoading() {
  if (this.accessibleDisabled) {
    return this.disabled || (this.isButton && this.loading);
  }
  return this.isButton && this.loading;
}

button state	accessible_disabled_button off	on
loading	aria-disabled="true", no native attribute	aria-disabled="true", no native attribute
disabled	native attribute	aria-disabled="true", no native attribute

Capybara's :button and :link_or_button selectors filter on the native property:

node_filter(:disabled, :boolean, default: false, skip_if: :all) { |node, value| !(value ^ node.disabled?) }

So an unavailable button still matches. Capybara clicks it, GlButton throws the event away, and the spec proceeds as though the action ran. It then fails later, somewhere else, with a message that points away from the real cause.

Note the loading row: this already happens today, in every configuration. It is not something the rollout of accessible_disabled_button will introduce — that flag only extends the same behaviour to explicitly disabled buttons.

Raised in #600158 (comment 3672623704).

Pipeline results

https://gitlab.com/gitlab-org/gitlab/-/pipelines/2751138065 ran the suite against this change:

	jobs
success	191
skipped	6
manual	1
failed	2

The two failed jobs (rspec-ee system pg17 12/16 and rspec-ee system pg17 es9 12/16) are the same single example in two shards: ee/spec/features/merge_request/user_sets_approvers_spec.rb:156. That example is fixed here.

Why that example broke

It was relying on the old behaviour by accident:

within(drawer_selector) do
  click_button 'Save changes'      # the approval rule drawer
end

click_on("Save changes")           # the merge request form's submit

While the rule saves, the drawer's button is :loading="isLoading" (ee/app/assets/javascripts/approvals/components/rule_drawer/create_rule.vue), so it renders aria-disabled="true" but stays in the DOM. The merge request edit form has a Save changes submit of its own (app/views/shared/issuable/_form.html.haml:60), so the page-level click_on matched two buttons and Capybara retried until the drawer closed. The ambiguity was doing the waiting.

Excluding the busy button removes the ambiguity, so click_on matched the form submit immediately and posted the form before the rule had been stored. The Capybara HTML artifact from the failed job confirms the end state: the browser is on the merge request page and it reads "Approval is optional", so no rule was ever applied.

The fix waits for the drawer to close, which rule_form.submit() only does after the save resolves:

within(drawer_selector) do
  click_button 'Save changes'
end
expect(page).to have_no_css(drawer_selector)

click_on("Save changes")

That is the idiom the sibling examples in the same file already use (user_sets_approvers_spec.rb:110). Applied to all three call sites that had the same pattern, not just the one CI caught, since the other two were relying on the same accident.

How to set up and validate locally

bundle exec rspec ee/spec/features/merge_request/user_sets_approvers_spec.rb
bundle exec rspec \
  spec/features/groups/runners/owner_manages_runners_spec.rb \
  spec/features/admin/runners/admin_manages_runners_spec.rb \
  spec/features/projects/runners/maintainer_manages_project_runners_spec.rb

Reproducing the original problem

spec/support/shared_examples/features/runners_shared_examples.rb no longer passes aria_disabled: false, because it no longer needs to. To see why the plain click_button "Resume" needs this change, widen the window it depends on:

// app/assets/javascripts/ci/runner/components/runner_pause_action.vue
} finally {
  await new Promise((resolve) => { setTimeout(resolve, 3000); });
  this.loading = false;
}

	result
without this change	2 examples, 2 failures — both expected not to find text "Paused"
with this change	2 examples, 0 failures

What to watch for

Two categories of spec could still be affected elsewhere:

1. have_button('X') assertions that run while the button happens to be busy.
2. Specs that click an unavailable button and expect nothing to happen — these now raise ElementNotFound after the wait, rather than silently passing.

Both are worth fixing on their own merits: each one is a spec that is currently clicking into the void, or asserting on a control the user cannot activate. The pipeline above suggests the number is very small.

Related

• !249594 (merged) — the opt-in filter this supersedes, now merged. Removed here.
• #600158 (closed) — the accessible_disabled_button rollout.
• https://gitlab.com/gitlab-org/quality/test-failure-issues/-/work_items/43855 — the flaky test that surfaced this.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.