Limit ARIA role check on buttons

Add test coverage to ensure that definitions and selectors for other
buttons aren't affected by the `enable_aria_role:` option.

The `:button` selector definition becomes too lax when
`Capybara.enable_aria_role = true`, which results in [false negatives
for `have_no_button`][false-negative] and false positives for
`have_button`.

The source is related to when that XPath is combined with other
selectors in the [Selector::Definition::Button][].

The lines were last changed as part of #2400, so this commit [partially
modifies some of those changes][comment].

[false-negative]: https://travis-ci.org/github/teamcapybara/capybara/jobs/746824288#L868-L871
[Selector::Definition::Button]: 30ec8c188d/lib/capybara/selector/definition/button.rb (L22-L25)
[comment]: https://github.com/teamcapybara/capybara/pull/2400#issuecomment-700864666?

lib/capybara/selector/definition/button.rb

@@ -3,33 +3,29 @@
 Capybara.add_selector(:button, locator_type: [String, Symbol]) do
   xpath(:value, :title, :type, :name) do |locator, **options|
     input_btn_xpath = XPath.descendant(:input)[XPath.attr(:type).one_of('submit', 'reset', 'image', 'button')]
-    btn_xpath = XPath.descendant(:button)
-    btn_xpath += XPath.descendant[XPath.attr(:role).equals('button')] if enable_aria_role
+    btn_xpaths = [XPath.descendant(:button)]
+    btn_xpaths << XPath.descendant[XPath.attr(:role).equals('button')] if enable_aria_role
     image_btn_xpath = XPath.descendant(:input)[XPath.attr(:type) == 'image']
 
     unless locator.nil?
       locator = locator.to_s
-      locator_matchers = XPath.attr(:id).equals(locator) |
-                         XPath.attr(:name).equals(locator) |
-                         XPath.attr(:value).is(locator) |
-                         XPath.attr(:title).is(locator) |
-                         (XPath.attr(:id) == XPath.anywhere(:label)[XPath.string.n.is(locator)].attr(:for))
-      locator_matchers |= XPath.attr(:'aria-label').is(locator) if enable_aria_label
-      locator_matchers |= XPath.attr(test_id) == locator if test_id
+      locator_matchers = combine_locators(locator, config: self)
 
       input_btn_xpath = input_btn_xpath[locator_matchers] + locate_label(locator).descendant(input_btn_xpath)
 
-      btn_xpath = btn_xpath[locator_matchers |
-                            XPath.string.n.is(locator) |
-                            XPath.descendant(:img)[XPath.attr(:alt).is(locator)]
-                           ] + locate_label(locator).descendant(btn_xpath)
+      btn_xpaths = btn_xpaths.map do |btn_xpath|
+        btn_xpath[locator_matchers |
+                  XPath.string.n.is(locator) |
+                  XPath.descendant(:img)[XPath.attr(:alt).is(locator)]
+                 ] + locate_label(locator).descendant(btn_xpath)
+      end
 
       alt_matches = XPath.attr(:alt).is(locator)
       alt_matches |= XPath.attr(:'aria-label').is(locator) if enable_aria_label
       image_btn_xpath = image_btn_xpath[alt_matches] + locate_label(locator).descendant(image_btn_xpath)
     end
 
-    %i[value title type].inject(input_btn_xpath.union(btn_xpath).union(image_btn_xpath)) do |memo, ef|
+    %i[value title type].inject([input_btn_xpath, *btn_xpaths, image_btn_xpath].inject(&:union)) do |memo, ef|
       memo.where(find_by_attr(ef, options[ef]))
     end
   end
@@ -51,4 +47,16 @@ Capybara.add_selector(:button, locator_type: [String, Symbol]) do
   describe_node_filters do |disabled: nil, **|
     ' that is disabled' if disabled == true
   end
+
+  def combine_locators(locator, config:)
+    [
+      XPath.attr(:id).equals(locator),
+      XPath.attr(:name).equals(locator),
+      XPath.attr(:value).is(locator),
+      XPath.attr(:title).is(locator),
+      (XPath.attr(:id) == XPath.anywhere(:label)[XPath.string.n.is(locator)].attr(:for)),
+      (XPath.attr(:'aria-label').is(locator) if config.enable_aria_label),
+      (XPath.attr(test_id) == locator if config.test_id)
+    ].compact.inject(&:|)
+  end
 end

lib/capybara/spec/session/has_button_spec.rb

@@ -46,9 +46,25 @@ Capybara::SpecHelper.spec '#has_button?' do
     expect(@session).to have_button('ARIA button', enable_aria_role: true)
   end
 
+  it 'should be true for a role=button within a label when enable_aria_role: true' do
+    expect(@session).to have_button('role=button within label', enable_aria_role: true)
+  end
+
   it 'should be false for role=button when enable_aria_role: false' do
     expect(@session).not_to have_button('ARIA button', enable_aria_role: false)
   end
+
+  it 'should be false for a role=button within a label when enable_aria_role: false' do
+    expect(@session).not_to have_button('role=button within label', enable_aria_role: false)
+  end
+
+  it 'should not affect other selectors when enable_aria_role: true' do
+    expect(@session).to have_button('Click me!', enable_aria_role: true)
+  end
+
+  it 'should not affect other selectors when enable_aria_role: false' do
+    expect(@session).to have_button('Click me!', enable_aria_role: false)
+  end
 end
 
 Capybara::SpecHelper.spec '#has_no_button?' do
@@ -81,7 +97,23 @@ Capybara::SpecHelper.spec '#has_no_button?' do
     expect(@session).to have_no_button('ARIA button', enable_aria_role: false)
   end
 
+  it 'should be true for role=button within a label when enable_aria_role: false' do
+    expect(@session).to have_no_button('role=button within label', enable_aria_role: false)
+  end
+
   it 'should be false for role=button when enable_aria_role: true' do
     expect(@session).not_to have_no_button('ARIA button', enable_aria_role: true)
   end
+
+  it 'should be false for a role=button within a label when enable_aria_role: true' do
+    expect(@session).not_to have_no_button('role=button within label', enable_aria_role: true)
+  end
+
+  it 'should not affect other selectors when enable_aria_role: true' do
+    expect(@session).to have_no_button('Junk button that does not exist', enable_aria_role: true)
+  end
+
+  it 'should not affect other selectors when enable_aria_role: false' do
+    expect(@session).to have_no_button('Junk button that does not exist', enable_aria_role: false)
+  end
 end

lib/capybara/spec/views/form.erb

@@ -462,6 +462,10 @@ New line after and before textarea tag
       button within label element
       <button></button>
     </label>
+    <label>
+      role=button within label element
+      <span role="button">with other text</span>
+    </label>
     <input type="button" disabled="disabled" value="Disabled button"/>
     <span role="button">ARIA button</span>
   </p>