Page method navigation result (#382)

* PageMethod: extract headers from last navigation

* Update test HTML file

* Paint it black

* Fix test, remove redundant test

* Update and simplify test

* Use handler's from_crawler in test

* Simplify check

* Update readme

Author: elacuesta

README.md

@@ -959,10 +959,10 @@ to see available methods.
 
 ### Impact on Response objects
 
-Certain `Response` attributes (e.g. `url`, `ip_address`) reflect the state after the last
-action performed on a page. If you issue a `PageMethod` with an action that results in
-a navigation (e.g. a `click` on a link), the `Response.url` attribute will point to the
-new URL, which might be different from the request's URL.
+Certain `Response` attributes (e.g. `url`, `ip_address`, `status`, `headers`) reflect the
+state after the last action performed on a page. If you issue a `PageMethod` with an action
+that results in a navigation (e.g. a `click` on a link), these attributes will point to the
+new page, which might be different from the request's URL.
 
 
 ## Handling page events

scrapy_playwright/handler.py

@@ -518,6 +518,13 @@ async def _download_request_with_page(
 
         start_time = time()
         response, download = await self._get_response_and_download(request, page, spider)
+
+        # page methods may navigate the main frame away from the original response
+        response = await self._maybe_apply_page_methods(
+            page=page, request=request, spider=spider, response=response
+        )
+
+        headers = Headers()
         if isinstance(response, PlaywrightResponse):
             await _set_redirect_meta(request=request, response=response)
             headers = Headers(await response.all_headers())
@@ -534,9 +541,7 @@ async def _download_request_with_page(
                     "scrapy_request_method": request.method,
                 },
             )
-            headers = Headers()
 
-        await self._apply_page_methods(page, request, spider)
         body_str = await _get_page_content(
             page=page,
             spider=spider,
@@ -672,7 +677,51 @@ async def _handle_response(response: PlaywrightResponse) -> None:
 
         return response, download if download else None
 
-    async def _apply_page_methods(self, page: Page, request: Request, spider: Spider) -> None:
+    async def _maybe_apply_page_methods(
+        self,
+        page: Page,
+        request: Request,
+        spider: Spider,
+        response: Optional[PlaywrightResponse],
+    ) -> Optional[PlaywrightResponse]:
+        """Run the request's page methods, returning the final Playwright response to use.
+
+        If a page method navigates the main frame away from the original response, the final Scrapy
+        response should have updated URL, body, status and headers. URL and body can be taken from
+        the Playwright Page, but status and headers need to be taken from the final Playwright
+        Response, which is not available as return value of page.goto().
+        """
+        if not request.meta.get("playwright_page_methods"):
+            return response
+
+        # track the most recent main-frame document navigation triggered by the page methods
+        last_navigation = response
+
+        def _track_navigation(navigation_response: PlaywrightResponse) -> None:
+            nonlocal last_navigation
+            if (
+                navigation_response.frame is page.main_frame
+                and navigation_response.request.is_navigation_request()
+                and navigation_response.request.resource_type == "document"
+            ):
+                last_navigation = navigation_response
+
+        page.on("response", _track_navigation)
+        try:
+            await self._run_page_methods(page, request, spider)
+        finally:
+            page.remove_listener("response", _track_navigation)
+
+        # use the final navigation response if it superseded the original one
+        if (
+            last_navigation is not response
+            and isinstance(last_navigation, PlaywrightResponse)
+            and last_navigation.url.rstrip("/") == page.url.rstrip("/")
+        ):
+            return last_navigation
+        return response
+
+    async def _run_page_methods(self, page: Page, request: Request, spider: Spider) -> None:
         context_name = request.meta.get("playwright_context")
         page_methods = request.meta.get("playwright_page_methods") or ()
         if isinstance(page_methods, dict):

tests/site/index.html

@@ -11,6 +11,7 @@
     <h1>Awesome site</h1>
     <p><a class="lorem_ipsum" href="lorem_ipsum.html">Lorem Ipsum</a></p>
     <p><a class="scroll" href="scroll.html">Infinite Scroll</a></p>
+    <p><a class="json" href="data/quotes1.json">Quotes JSON</a></p>
 </div>
 </body>
 </html>

tests/tests_asyncio/test_page_methods.py

@@ -1,3 +1,4 @@
+import json
 import logging
 import platform
 import subprocess
@@ -6,7 +7,6 @@
 
 import pytest
 from scrapy import Spider, Request
-from scrapy.http.response.html import HtmlResponse
 
 from playwright.async_api import Page
 from scrapy_playwright.page import PageMethod
@@ -79,24 +79,31 @@ async def test_mixed(self):
 
     @allow_windows
     async def test_page_method_navigation(self):
+        """A PageMethod that navigates to a different page must produce a response
+        whose headers and status match the final page's body, not the initial one.
+        """
         async with make_handler({"PLAYWRIGHT_BROWSER_TYPE": self.browser_type}) as handler:
             req = Request(
                 url=self.static_server.urljoin("/index.html"),
                 meta={
                     "playwright": True,
-                    "playwright_page_methods": [PageMethod("click", "a.lorem_ipsum")],
+                    "playwright_page_methods": [PageMethod("click", "a.json")],
                 },
             )
             resp = await handler._download_request(req, Spider("foo"))
 
-            assert isinstance(resp, HtmlResponse)
-            assert resp.request is req
-            assert resp.url == self.static_server.urljoin("/lorem_ipsum.html")
-            assert resp.status == 200
-            assert "playwright" in resp.flags
-            assert resp.css("title::text").get() == "Lorem Ipsum"
-            text = resp.css("p::text").get()
-            assert text == "Lorem ipsum dolor sit amet, consectetur adipiscing elit."
+        assert resp.request is req
+        assert resp.url == self.static_server.urljoin("/data/quotes1.json")
+        assert resp.status == 200
+        assert "playwright" in resp.flags
+        # headers must match the final (JSON) page, not the initial HTML page
+        assert resp.headers.get("Content-Type", b"").startswith(b"application/json")
+        # parse body and verify it's JSON, not HTML
+        body = json.loads(resp.css("pre::text").get())
+        assert isinstance(body, dict)
+        assert isinstance(body.get("quotes"), list)
+        assert body.get("has_next") is True
+        assert body.get("page") == 1
 
     @allow_windows
     async def test_page_method_infinite_scroll(self):

tests/tests_asyncio/test_playwright_requests.py

@@ -44,8 +44,12 @@ async def test_basic_response(self):
             req = Request(self.static_server.urljoin("/index.html"), meta={"playwright": True})
             resp = await handler._download_request(req, spider)
             assert_correct_response(resp, req)
+            assert resp.css("a::text").getall() == [
+                "Lorem Ipsum",
+                "Infinite Scroll",
+                "Quotes JSON",
+            ]
             assert resp.ip_address == ip_address(self.static_server.address)
-            assert resp.css("a::text").getall() == ["Lorem Ipsum", "Infinite Scroll"]
 
             # at least one log record has a spider attribute
             # (records sent before spider_opened will not have it)

tests/tests_twisted/test_mixed_requests.py

@@ -33,7 +33,9 @@ def tearDown(self):
     def test_download_request(self):
         def _check_response(response: Response, request: Request) -> None:
             self.assertIsInstance(response, Response)
-            self.assertEqual(response.css("a::text").getall(), ["Lorem Ipsum", "Infinite Scroll"])
+            self.assertEqual(
+                response.css("a::text").getall(), ["Lorem Ipsum", "Infinite Scroll", "Quotes JSON"]
+            )
             self.assertEqual(response.url, request.url)
             self.assertEqual(response.status, 200)
             if request.meta.get("playwright"):