From ba33a6bae72eb3998130ac1ff36b3642ee27e4f0 Mon Sep 17 00:00:00 2001
From: Thomas Walpole <twalpole@gmail.com>
Date: Thu, 4 Jan 2018 17:38:22 -0800
Subject: [PATCH] Don't check node ancestors for CSS visibility setting

---
 spec/node_spec.rb | 24 ++++++++++++++++++++++++
 src/capybara.js   | 16 +++++++++++-----
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/spec/node_spec.rb b/spec/node_spec.rb
index 88cfe78..4b6f9b1 100644
--- a/spec/node_spec.rb
+++ b/spec/node_spec.rb
@@ -31,6 +31,18 @@ describe Capybara::Webkit::Node do
               <input type="checkbox" name="falsecheckedbox" value="3" checked="false"/>
               <input type="text" name="styled" style="font-size: 150%;"/>
             </form>
+
+            <div id="visibility_wrapper" style="visibility: hidden">
+              <div id="hidden">Hidden</div>
+              <div id="visible" style="visibility: visible">Visibile</div>
+              <div style="visibility: visible">
+                <div id="nested_visible">Nested Visibile</div>
+              </div>
+            </div>
+
+            <div id="display_none" style="display: none">
+              <div id="not_displayed" style="visibility: visible; display: block;">Should not be displayed</div>
+            </div>
           </body>
         </html>
       HTML
@@ -62,5 +74,17 @@ describe Capybara::Webkit::Node do
         expect(input["style"]).to eq "font-size: 150%;"
       end
     end
+
+    context "Node#visible" do
+      it "correctly analyzes visibility CSS" do
+        expect(driver.find_css('#hidden').first.visible?).to be false
+        expect(driver.find_css('#visible').first.visible?).to be true
+        expect(driver.find_css('#nested_visible').first.visible?).to be true
+      end
+
+      it "correctly analyzes display: none CSS" do
+        expect(driver.find_css('#not_displayed').first.visible?).to be false
+      end
+    end
   end
 end
diff --git a/src/capybara.js b/src/capybara.js
index 3365d05..86c11b0 100644
--- a/src/capybara.js
+++ b/src/capybara.js
@@ -249,12 +249,18 @@ Capybara = {
   },
 
   isNodeVisible: function(node) {
-    while (node) {
-      var style = node.ownerDocument.defaultView.getComputedStyle(node, null);
-      if (style.getPropertyValue('display') == 'none' || style.getPropertyValue('visibility') == 'hidden')
-        return false;
+    var style = node.ownerDocument.defaultView.getComputedStyle(node, null);
+    // Only check computed visibility style on current node since it
+    // will inherit from nearest ancestor with a setting and overrides
+    // any farther ancestors
+    if (style.getPropertyValue('visibility') == 'hidden' || style.getPropertyValue('display') == 'none')
+      return false;
 
-      node = node.parentElement;
+    // Must check CSS display setting for all ancestors
+    while (node = node.parentElement) {
+      style = node.ownerDocument.defaultView.getComputedStyle(node, null);
+      if (style.getPropertyValue('display') == 'none' )
+        return false;
     }
     return true;
   },