Merge pull request #175 from rubycdp/refactor

* Introduce wait_for_stop_moving and moving?

Author: route

README.md

@@ -50,6 +50,7 @@ Web design by [Evrone](https://evrone.com/), what else
 * [Frame](https://github.com/rubycdp/ferrum#frame)
 * [Dialog](https://github.com/rubycdp/ferrum#dialog)
 * [Animation](https://github.com/rubycdp/ferrum#animation)
+* [Node](https://github.com/rubycdp/ferrum#node)
 * [Thread safety](https://github.com/rubycdp/ferrum#thread-safety)
 * [Development](https://github.com/rubycdp/ferrum#development)
 * [Contributing](https://github.com/rubycdp/ferrum#contributing)
@@ -979,6 +980,7 @@ end
 browser.go_to("https://google.com")
 ```
 
+
 ## Animation
 
 You can slow down or speed up CSS animations.
@@ -1002,6 +1004,32 @@ browser.playback_rate # => 2000
 ```
 
 
+## Node
+
+#### node? : `Boolean`
+#### frame_id
+#### frame
+#### focus
+#### focusable?
+#### moving? : `Boolean`
+#### wait_for_stop_moving
+#### blur
+#### type
+#### click
+#### hover
+#### select_file
+#### at_xpath
+#### at_css
+#### xpath
+#### css
+#### text
+#### inner_text
+#### value
+#### property
+#### attribute
+#### evaluate
+
+
 ## Thread safety ##
 
 Ferrum is fully thread-safe. You can create one browser or a few as you wish and

lib/ferrum.rb

@@ -57,7 +57,7 @@ def initialize(message = "Browser is dead or given window is closed")
     end
   end
 
-  class NodeIsMovingError < Error
+  class NodeMovingError < Error
     def initialize(node, prev, current)
       @node, @prev, @current = node, prev, current
       super(message)
@@ -70,6 +70,12 @@ def message
     end
   end
 
+  class CoordinatesNotFoundError < Error
+    def initialize(message = "Could not compute content quads")
+      super
+    end
+  end
+
   class BrowserError < Error
     attr_reader :response
 

lib/ferrum/browser/client.rb

@@ -90,6 +90,8 @@ def raise_browser_error(error)
           raise NoExecutionContextError.new(error)
         when "No target with given id found"
           raise NoSuchPageError
+        when /Could not compute content quads/
+          raise CoordinatesNotFoundError
         else
           raise BrowserError.new(error)
         end

lib/ferrum/node.rb

@@ -2,8 +2,8 @@
 
 module Ferrum
   class Node
-    MOVING_WAIT = ENV.fetch("FERRUM_NODE_MOVING_WAIT", 0.01).to_f
-    MOVING_ATTEMPTS = ENV.fetch("FERRUM_NODE_MOVING_ATTEMPTS", 50).to_i
+    MOVING_WAIT_DELAY = ENV.fetch("FERRUM_NODE_MOVING_WAIT", 0.01).to_f
+    MOVING_WAIT_ATTEMPTS = ENV.fetch("FERRUM_NODE_MOVING_ATTEMPTS", 50).to_i
 
     attr_reader :page, :target_id, :node_id, :description, :tag_name
 
@@ -37,6 +37,19 @@ def focusable?
       e.message == "Element is not focusable" ? false : raise
     end
 
+    def wait_for_stop_moving(delay: MOVING_WAIT_DELAY, attempts: MOVING_WAIT_ATTEMPTS)
+      Ferrum.with_attempts(errors: NodeMovingError, max: attempts, wait: 0) do
+        previous, current = get_content_quads_with(delay: delay)
+        raise NodeMovingError.new(self, previous, current) if previous != current
+        current
+      end
+    end
+
+    def moving?(delay: MOVING_WAIT_DELAY)
+      previous, current = get_content_quads_with(delay: delay)
+      previous == current
+    end
+
     def blur
       tap { evaluate("this.blur()") }
     end
@@ -131,44 +144,36 @@ def inspect
     end
 
     def find_position(x: nil, y: nil, position: :top)
-      prev = get_content_quads
-
-      # FIXME: Case when a few quads returned
-      points = Ferrum.with_attempts(errors: NodeIsMovingError, max: MOVING_ATTEMPTS, wait: 0) do
-        sleep(MOVING_WAIT)
-        current = get_content_quads
-
-        if current != prev
-          error = NodeIsMovingError.new(self, prev, current)
-          prev = current
-          raise(error)
-        end
-
-        current
-      end.map { |q| to_points(q) }.first
-
+      points = wait_for_stop_moving.map { |q| to_points(q) }.first
       get_position(points, x, y, position)
-    rescue Ferrum::BrowserError => e
-      return raise unless e.message&.include?("Could not compute content quads")
-
-      find_position_via_js
+    rescue CoordinatesNotFoundError
+      x, y = get_bounding_rect_coordinates
+      raise if x == 0 && y == 0
+      [x, y]
     end
 
     private
 
-    def find_position_via_js
-      [
-        evaluate("this.getBoundingClientRect().left + window.pageXOffset + (this.offsetWidth / 2)"), # x
-        evaluate("this.getBoundingClientRect().top + window.pageYOffset + (this.offsetHeight / 2)") # y
-      ]
+    def get_bounding_rect_coordinates
+      evaluate <<~JS
+        [this.getBoundingClientRect().left + window.pageXOffset + (this.offsetWidth / 2),
+         this.getBoundingClientRect().top + window.pageYOffset + (this.offsetHeight / 2)]
+      JS
     end
 
     def get_content_quads
       quads = page.command("DOM.getContentQuads", nodeId: node_id)["quads"]
-      raise "Node is either not visible or not an HTMLElement" if quads.size == 0
+      raise CoordinatesNotFoundError, "Node is either not visible or not an HTMLElement" if quads.size == 0
       quads
     end
 
+    def get_content_quads_with(delay: MOVING_WAIT_DELAY)
+      previous = get_content_quads
+      sleep(delay)
+      current = get_content_quads
+      [previous, current]
+    end
+
     def get_position(points, offset_x, offset_y, position)
       x = y = nil
 

spec/browser_spec.rb

@@ -508,7 +508,9 @@ module Ferrum
     it "does not run into content quads error" do
       browser.go_to("/ferrum/index")
 
-      allow_any_instance_of(Node).to receive(:get_content_quads).and_raise(Ferrum::BrowserError, "message" => "Could not compute content quads")
+      allow_any_instance_of(Node).to receive(:get_content_quads)
+                                      .and_raise(Ferrum::CoordinatesNotFoundError,
+                                                 "Could not compute content quads")
 
       browser.at_xpath("//a[text() = 'JS redirect']").click
       expect(browser.body).to include("Hello world")

spec/node_spec.rb

@@ -17,16 +17,20 @@ module Ferrum
       expect { node.text }.to raise_error(Ferrum::NodeNotFoundError)
     end
 
-    it "raises an error if the element is not visible", skip: true do
+    it "raises an error if the element is not visible" do
       browser.go_to("/ferrum/index")
+
       browser.execute <<~JS
         document.querySelector("a[href=js_redirect]").style.display = "none"
       JS
+
+      sleep 0.2 # Wait for node to disappear
+
       expect {
         browser.at_xpath("//a[text()='JS redirect']").click
       }.to raise_error(
-        Ferrum::BrowserError,
-        "Could not compute content quads."
+        Ferrum::CoordinatesNotFoundError,
+        "Could not compute content quads"
       )
     end