Merge pull request #1049 from thoughtbot/visibility_issue

Fix issue with CSS `visibility` style - Issue #1048

.travis.yml

@@ -2,10 +2,9 @@ sudo: required
 dist: trusty
 language: ruby
 rvm:
-  - 1.9.3
   - 2.2
   - 2.4
-  - jruby-19mode
+  - 2.5
 notifications:
   email: false
 script: xvfb-run bundle exec rake
@@ -27,6 +26,8 @@ matrix:
     - rvm: 1.9.3
       gemfile: gemfiles/2.15.gemfile
       env: QMAKE=/usr/lib/x86_64-linux-gnu/qt4/bin/qmake
+    - rvm: jruby-19mode
+      gemfile: gemfiles/2.15.gemfile
     - rvm: 2.3.3
       gemfile: gemfiles/master.gemfile
     - rvm: jruby-9.1.13.0
@@ -34,7 +35,7 @@ matrix:
   allow_failures:
     - gemfile: gemfiles/master.gemfile
 gemfile:
-  - gemfiles/2.15.gemfile
+  - Gemfile
 before_install:
   - gem install bundler
 install: bundle

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

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;
   },