test: clean up skipped specs and fix the bugs they hid

AGENTS.md

@@ -28,9 +28,10 @@ The suite boots a real Chrome and a Sinatra/Puma test app (`spec/support/applica
 
 - **Prefer system/feature specs**: drive a real browser against the test app and assert on observable behavior.
   They live in `spec/` next to the feature, e.g. `spec/page_spec.rb`, `spec/network/`.
-- **Unit specs go in `spec/unit/` and nowhere else.** A spec is a unit spec if it stubs or mocks Ferrum's own
-  objects (`allow(...).to receive`, `and_raise`, `and_wrap_original`), builds objects with `allocate` or
-  `instance_variable_set`, or tests one class in isolation. Mirror the `lib/` path:
+- **Unit specs go in `spec/unit/` and nowhere else.** A unit spec tests one class in isolation or its internals:
+  it builds objects with `allocate` or `instance_variable_set`, or asserts on internal calls and state. A spec that
+  drives the browser and asserts observable behavior stays a feature spec even if it stubs one call, e.g. to
+  simulate a Chrome failure. Mirror the `lib/` path:
   `lib/ferrum/contexts.rb` -> `spec/unit/contexts_spec.rb`, `lib/ferrum/client/web_socket.rb` ->
   `spec/unit/client/web_socket_spec.rb`. Never add them to the feature spec files in `spec/`.
 - Don't test private methods; cover them through the public API.

CHANGELOG.md

@@ -46,6 +46,9 @@
 - Behind a proxy with credentials, `request.abort` in `on(:request)` and `network.blacklist`/`whitelist` were ignored
   because Ferrum's own proxy handler continued every request first. It now only continues requests nobody else handles [#332]
 - `mouse.click(modifiers:)` sent its `mousemove` without the modifiers [#581]
+- When Chrome can't compute an element's content quads, `node.click` falls back to its bounding rect, but it added the
+  page scroll offset and missed the element on a scrolled page. It clicks in viewport coordinates now
+- `Proxy#rotate` to an upstream proxy failed every HTTPS request with `ERR_TUNNEL_CONNECTION_FAILED`
 
 ### Removed
 - `Browser::Process#default_user_agent`, `#browser_version`, `#protocol_version`, `#v8_version` and `#webkit_version`,

lib/ferrum/node.rb

@@ -531,8 +531,8 @@ def exists?
 
     def bounding_rect_coordinates
       evaluate <<~JS
-        [this.getBoundingClientRect().left + window.pageXOffset + (this.offsetWidth / 2),
-         this.getBoundingClientRect().top + window.pageYOffset + (this.offsetHeight / 2)]
+        [this.getBoundingClientRect().left + (this.offsetWidth / 2),
+         this.getBoundingClientRect().top + (this.offsetHeight / 2)]
       JS
     end
 

lib/ferrum/proxy.rb

@@ -134,19 +134,19 @@ def do_CONNECT(req, res)
 
           if proxy
             @logger.debug("CONNECT: sending a Request-Line")
-            os << proxy_request_line << CRLF
+            os << proxy_request_line << WEBrick::CRLF
             @logger.debug("CONNECT: > #{proxy_request_line}")
             if credentials
               @logger.debug("CONNECT: sending credentials")
-              os << "Proxy-Authorization: " << credentials << CRLF
+              os << "Proxy-Authorization: " << credentials << WEBrick::CRLF
             end
-            os << CRLF
-            proxy_status_line = os.gets(LF)
+            os << WEBrick::CRLF
+            proxy_status_line = os.gets(WEBrick::LF)
             @logger.debug("CONNECT: read Status-Line from the upstream server")
             @logger.debug("CONNECT: < #{proxy_status_line}")
             if %r{^HTTP/\d+\.\d+\s+200\s*} =~ proxy_status_line
-              while line = os.gets(LF)
-                break if /\A(#{CRLF}|#{LF})\z/om =~ line
+              while line = os.gets(WEBrick::LF)
+                break if /\A(#{WEBrick::CRLF}|#{WEBrick::LF})\z/om =~ line
               end
             else
               raise WEBrick::HTTPStatus::BadGateway
@@ -170,7 +170,7 @@ def do_CONNECT(req, res)
 
           # Should clear request-line not to send the response twice.
           # see: HTTPServer#run
-          req.parse(NullReader) rescue nil
+          req.parse(WEBrick::NullReader) rescue nil
         end
 
         begin

sig/ferrum/node.rbs

@@ -82,7 +82,7 @@ module Ferrum
 
     private
 
-    def bounding_rect_coordinates: () -> untyped
+    def bounding_rect_coordinates: () -> [Numeric, Numeric]
 
     def content_quads: () -> untyped
 

spec/browser_spec.rb

@@ -583,24 +583,28 @@ def proxy_tunnel(socket, request, browser_url)
         end
       end
 
-      context "with rotation", skip: "Think how to make it working on CI" do
-        it "works after disposing context" do
+      context "with rotation" do
+        let(:upstream) { Ferrum::Proxy.start(user: "user", password: "pa$$") }
+
+        after { upstream.stop }
+
+        it "forwards connections to the upstream proxy in a new context", :launch do
           browser = Ferrum::Browser.new(
-            proxy: { host: proxy.host, port: proxy.port, **options }
+            proxy: { host: proxy.host, port: proxy.port }
           )
 
-          proxy.rotate(host: "host", port: 0, user: "user", password: "password")
+          proxy.rotate(host: upstream.host, port: upstream.port, user: "user", password: "pa$$")
           browser.create_page(new_context: true) do |page|
-            page.go_to("https://api.ipify.org?format=json")
+            page.go_to("https://example.com")
             expect(page.network.status).to eq(200)
-            expect(page.body).to include("x.x.x.x")
+            expect(page.body).to include("Example Domain")
           end
 
-          proxy.rotate(host: "host", port: 0, user: "user", password: "password")
+          proxy.rotate(host: upstream.host, port: upstream.port, user: "user", password: "wrong")
           browser.create_page(new_context: true) do |page|
-            page.go_to("https://api.ipify.org?format=json")
-            expect(page.network.status).to eq(200)
-            expect(page.body).to include("x.x.x.x")
+            expect { page.go_to("https://example.com") }.to raise_error(
+              Ferrum::StatusError, /ERR_TUNNEL_CONNECTION_FAILED/
+            )
           end
         ensure
           browser&.quit
@@ -631,9 +635,6 @@ def proxy_tunnel(socket, request, browser_url)
         let(:save_path) { "/tmp/ferrum" }
 
         it "saves an attachment" do
-          # Also https://github.com/puppeteer/puppeteer/issues/10161
-          skip "https://bugs.chromium.org/p/chromium/issues/detail?id=1444729"
-
           browser.go_to("/#{filename}")
           browser.downloads.wait
 

spec/frame_spec.rb

@@ -152,32 +152,20 @@
     end
   end
 
-  it "supports clicking in a frame", skip: true do
-    page.go_to
-    page.execute <<-JS
-      document.body.innerHTML += "<iframe src='/click_test' name='frame'>"
-    JS
-    sleep 0.5
-    frame = page.at_xpath("//iframe[@name = 'frame']").frame
-
-    log = frame.at_css("#log")
-    frame.at_css("#one").click
-    expect(log.text).to eq("one")
-  end
-
-  it "supports clicking in a frame with padding", skip: true do
+  it "supports clicking in a frame with padding" do
     page.go_to
     page.execute <<-JS
       document.body.innerHTML += "<iframe src='/click_test' name='padded_frame' style='padding:100px;'>"
     JS
+    sleep 0.5
     frame = page.at_xpath("//iframe[@name = 'padded_frame']").frame
 
     log = frame.at_css("#log")
-    frame.at_css("#one").click
+    frame.at_css("#one").scroll_into_view.click
     expect(log.text).to eq("one")
   end
 
-  it "supports clicking in a frame nested in a frame", skip: true do
+  it "supports clicking in a frame nested in a frame" do
     page.go_to
 
     # The padding on the frame here is to differ the sizes of the two
@@ -193,7 +181,7 @@
 
     inner_frame = page.frame_by(name: "inner_frame")
     log = inner_frame.at_css("#log")
-    inner_frame.at_css("#one").click
+    inner_frame.at_css("#one").scroll_into_view.click
     expect(log.text).to eq("one")
   end
 

spec/headers_spec.rb

@@ -83,84 +83,4 @@
       expect(browser.body).to include("X_CUSTOM_HEADER: 1")
     end
   end
-
-  context "multiple windows", skip: true do
-    it "persists headers across popup windows" do
-      browser.headers.set(
-        "Cookie" => "foo=bar",
-        "Host" => "foo.com",
-        "User-Agent" => "foo"
-      )
-      browser.go_to("/popup_headers")
-      browser.at_xpath("//a[text()='pop up']").click
-
-      page, = browser.windows(:last)
-
-      expect(page.body).to include("USER_AGENT: foo")
-      expect(page.body).to include("COOKIE: foo=bar")
-      expect(page.body).to include("HOST: foo.com")
-    end
-
-    it "sets headers in existing windows" do
-      page = browser.create_page
-      page.headers.set(
-        "Cookie" => "foo=bar",
-        "Host" => "foo.com",
-        "User-Agent" => "foo"
-      )
-      page.goto("/headers")
-      expect(page.body).to include("USER_AGENT: foo")
-      expect(page.body).to include("COOKIE: foo=bar")
-      expect(page.body).to include("HOST: foo.com")
-
-      browser.switch_to_window browser.windows.last
-      browser.go_to("/headers")
-      expect(browser.body).to include("USER_AGENT: foo")
-      expect(browser.body).to include("COOKIE: foo=bar")
-      expect(browser.body).to include("HOST: foo.com")
-    end
-
-    it "keeps temporary headers local to the current window" do
-      browser.create_page
-      browser.headers.add("X-Custom-Header" => "1", permanent: false)
-
-      browser.switch_to_window browser.windows.last
-      browser.go_to("/headers")
-      expect(browser.body).not_to include("X_CUSTOM_HEADER: 1")
-
-      browser.switch_to_window browser.windows.first
-      browser.go_to("/headers")
-      expect(browser.body).to include("X_CUSTOM_HEADER: 1")
-    end
-
-    it "does not mix temporary headers with permanent ones when propagating to other windows" do
-      browser.create_page
-      browser.headers.add("X-Custom-Header" => "1", permanent: false)
-      browser.headers.add("Host" => "foo.com")
-
-      browser.switch_to_window browser.windows.last
-      browser.go_to("/headers")
-      expect(browser.body).to include("HOST: foo.com")
-      expect(browser.body).not_to include("X_CUSTOM_HEADER: 1")
-
-      browser.switch_to_window browser.windows.first
-      browser.go_to("/headers")
-      expect(browser.body).to include("HOST: foo.com")
-      expect(browser.body).to include("X_CUSTOM_HEADER: 1")
-    end
-
-    it "does not propagate temporary headers to new windows" do
-      browser.go_to
-      browser.headers.add("X-Custom-Header" => "1", permanent: false)
-      browser.create_page
-
-      browser.switch_to_window browser.windows.last
-      browser.go_to("/headers")
-      expect(browser.body).not_to include("X_CUSTOM_HEADER: 1")
-
-      browser.switch_to_window browser.windows.first
-      browser.go_to("/headers")
-      expect(browser.body).to include("X_CUSTOM_HEADER: 1")
-    end
-  end
 end

spec/mouse_spec.rb

@@ -8,14 +8,14 @@
       expect(browser.body).to include("x: 100, y: 150")
     end
 
-    it "has no trouble clicking elements when the size of a document changes", skip: true do
+    it "has no trouble clicking elements when the size of a document changes" do
       browser.go_to("/long_page")
-      browser.at_css("#penultimate").click
+      browser.at_css("#penultimate").scroll_into_view.click
       browser.execute <<~JS
         el = document.getElementById("penultimate")
         el.parentNode.removeChild(el)
       JS
-      browser.at_xpath("//a[text() = 'Phasellus blandit velit']").click
+      browser.at_xpath("//a[text() = 'Phasellus blandit velit']").scroll_into_view.click
       expect(browser.body).to include("Hello")
     end
 
@@ -142,7 +142,7 @@
     end
   end
 
-  context "mouse support", skip: true do
+  context "mouse support" do
     before do
       browser.go_to("/click_test")
     end
@@ -152,104 +152,19 @@
       browser.reset
     end
 
-    it "scrolls around so that elements can be clicked" do
+    it "clicks elements scrolled into view" do
       browser.resize(width: 200, height: 200)
       log = browser.at_css("#log")
 
       instructions = %w[one four one two three]
       instructions.each do |instruction|
-        browser.at_css("##{instruction}").click
-        browser.screenshot(path: "a.png")
+        browser.at_css("##{instruction}").scroll_into_view.click
         expect(log.text).to eq(instruction)
       end
     end
 
-    it "fixes some weird layout issue that we are not entirely sure about the reason for" do
-      browser.go_to("/datepicker")
-      browser.at_css("#datepicker").set("2012-05-11")
-      browser.at_xpath("//a[text() = 'some link']").click
-    end
-
     it "can click an element inside an svg" do
       expect { browser.at_css("#myrect").click }.not_to raise_error
     end
-
-    context "with #two overlapping #one" do
-      before do
-        browser.execute <<-JS
-          var two = document.getElementById("two")
-          two.style.position = "absolute"
-          two.style.left     = "0px"
-          two.style.top      = "0px"
-        JS
-      end
-
-      it "detects if an element is obscured when clicking" do
-        expect do
-          browser.at_css("#one").click
-        end.to raise_error(Ferrum::MouseEventFailed) { |error|
-          expect(error.selector).to eq("html body div#two.box")
-          expect(error.message).to include("[200.0, 200.0]")
-        }
-      end
-
-      it "clicks in the center of an element" do
-        expect do
-          browser.at_css("#one").click
-        end.to raise_error(Ferrum::MouseEventFailed) { |error|
-          expect(error.position).to eq([200, 200])
-        }
-      end
-
-      it "clicks in the center of an element within the viewport, if part is outside the viewport" do
-        browser.resize(width: 200, height: 200)
-
-        expect do
-          browser.at_css("#one").click
-        end.to raise_error(Ferrum::MouseEventFailed) { |error|
-          expect(error.position.first).to eq(100)
-        }
-      end
-    end
-
-    context "with #svg overlapping #one" do
-      before do
-        browser.execute <<-JS
-          var two = document.getElementById("svg")
-          two.style.position = "absolute"
-          two.style.left     = "0px"
-          two.style.top      = "0px"
-        JS
-      end
-
-      it "detects if an element is obscured when clicking" do
-        expect do
-          browser.at_css("#one").click
-        end.to raise_error(Ferrum::MouseEventFailed) { |error|
-          expect(error.selector).to eq("html body svg#svg.box")
-          expect(error.message).to include("[200.0, 200.0]")
-        }
-      end
-    end
-
-    context "with image maps", skip: true do
-      before { browser.go_to("/image_map") }
-
-      it "can click" do
-        browser.at_css("map[name=testmap] area[shape=circle]").click
-        expect(browser).to have_css("#log", text: "circle clicked")
-        browser.at_css("map[name=testmap] area[shape=rect]").click
-        expect(browser).to have_css("#log", text: "rect clicked")
-      end
-
-      it "doesn't click if the associated img is hidden" do
-        expect do
-          browser.at_css("map[name=testmap2] area[shape=circle]").click
-        end.to raise_error(Ferrum::ElementNotFound)
-        expect do
-          browser.at_css("map[name=testmap2] area[shape=circle]", visible: false).click
-        end.to raise_error(Ferrum::MouseEventFailed)
-      end
-    end
   end
 end