[widget audit] toga.WebView

.github/workflows/ci.yml

@@ -240,7 +240,9 @@ jobs:
       uses: actions/setup-python@v4.6.1
       if: ${{ matrix.setup-python }}
       with:
-        python-version: "3.X"
+        # We're not using 3.11 yet, because the testbed's ProxyEventLoop isn't
+        # compatible with it (https://github.com/beeware/toga/issues/1982).
+        python-version: "3.10"
     - name: Install dependencies
       run: |
         ${{ matrix.pre-command }}

android/src/toga_android/widgets/webview.py

@@ -1,25 +1,34 @@
-import asyncio
-import base64
+import json
 
 from travertino.size import at_least
 
-from ..libs.android.view import View__MeasureSpec
+from toga.widgets.webview import JavaScriptResult
+
 from ..libs.android.webkit import ValueCallback, WebView as A_WebView, WebViewClient
 from .base import Widget
 
 
 class ReceiveString(ValueCallback):
-    def __init__(self, fn=None):
+    def __init__(self, future, on_result):
         super().__init__()
-        self._fn = fn
+        self.future = future
+        self.on_result = on_result
 
     def onReceiveValue(self, value):
-        if self._fn:
-            if value is None:
-                self._fn(None)
-            else:
-                # Ensure we send a string to the function.
-                self._fn(value.toString())
+        # If the evaluation fails, a message is written to Logcat, but the value sent to
+        # the callback will be "null", with no way to distinguish it from an actual null
+        # return value.
+        result = json.loads(value)
+
+        # Because this method is called directly from the Android event loop, calling
+        # set_result on a timed-out future would crash the whole testbed with an
+        # InvalidStateError.
+        if self.future.cancelled():  # pragma: nocover
+            pass
+        else:
+            self.future.set_result(result)
+            if self.on_result:
+                self.on_result(result)
 
 
 class WebView(Widget):
@@ -28,59 +37,50 @@ def create(self):
         # Set a WebViewClient so that new links open in this activity,
         # rather than triggering the phone's web browser.
         self.native.setWebViewClient(WebViewClient())
-        # Enable JS.
-        self.native.getSettings().setJavaScriptEnabled(True)
-
-    def set_on_key_down(self, handler):
-        # Android isn't a platform that usually has a keyboard attached, so this is unimplemented for now.
-        self.interface.factory.not_implemented("WebView.set_on_key_down()")
-
-    def set_on_webview_load(self, handler):
-        # This requires subclassing WebViewClient, which is not yet possible with rubicon-java.
-        self.interface.factory.not_implemented("WebView.set_on_webview_load()")
 
-    def get_dom(self):
-        # Android has no straightforward way to get the DOM from the browser synchronously.
-        self.interface.factory.not_implemented("WebView.get_dom()")
+        self.settings = self.native.getSettings()
+        self.default_user_agent = self.settings.getUserAgentString()
+        self.settings.setJavaScriptEnabled(True)
 
     def get_url(self):
-        return self.native.getUrl()
-
-    def set_url(self, value):
-        if value:
-            self.native.loadUrl(str(value))
+        url = self.native.getUrl()
+        if url == "about:blank" or url.startswith("data:"):
+            return None
+        else:
+            return url
+
+    def set_url(self, value, future=None):
+        if value is None:
+            value = "about:blank"
+        self.native.loadUrl(value)
+
+        # Detecting when the load is complete requires subclassing WebViewClient
+        # (https://github.com/beeware/toga/issues/1020).
+        if future:
+            future.set_result(None)
 
     def set_content(self, root_url, content):
-        # Android WebView lacks an underlying set_content() primitive, so we navigate to
-        # a data URL. This means we ignore the root_url parameter.
-        data_url = "data:text/html; charset=utf-8; base64," + base64.b64encode(
-            content.encode("utf-8")
-        ).decode("ascii")
-        self.set_url(data_url)
+        # There is a loadDataWithBaseURL method, but it's inconsistent about whether
+        # getUrl returns the given URL or a data: URL. Rather than support this feature
+        # intermittently, it's better to not support it at all.
+        self.native.loadData(content, "text/html", "utf-8")
+
+    def get_user_agent(self):
+        return self.settings.getUserAgentString()
 
     def set_user_agent(self, value):
-        if value is not None:
-            self.native.getSettings().setUserAgentString(value)
+        self.settings.setUserAgentString(
+            self.default_user_agent if value is None else value
+        )
+
+    def evaluate_javascript(self, javascript, on_result=None):
+        result = JavaScriptResult()
 
-    async def evaluate_javascript(self, javascript):
-        js_value = asyncio.Future()
         self.native.evaluateJavascript(
-            str(javascript), ReceiveString(js_value.set_result)
+            javascript, ReceiveString(result.future, on_result)
         )
-        return await js_value
-
-    def invoke_javascript(self, javascript):
-        self.native.evaluateJavascript(str(javascript), ReceiveString())
+        return result
 
     def rehint(self):
         self.interface.intrinsic.width = at_least(self.interface._MIN_WIDTH)
-        # Refuse to call measure() if widget has no container, i.e., has no LayoutParams.
-        # Android's measure() throws NullPointerException if the widget has no LayoutParams.
-        if not self.native.getLayoutParams():
-            return
-        self.native.measure(
-            View__MeasureSpec.UNSPECIFIED,
-            View__MeasureSpec.UNSPECIFIED,
-        )
-        self.interface.intrinsic.width = at_least(self.native.getMeasuredWidth())
-        self.interface.intrinsic.height = self.native.getMeasuredHeight()
+        self.interface.intrinsic.height = at_least(self.interface._MIN_HEIGHT)

android/tests_backend/probe.py

@@ -11,9 +11,12 @@ def assert_font_family(self, expected):
         else:
             assert actual == expected
 
-    async def redraw(self, message=None):
+    async def redraw(self, message=None, delay=None):
         """Request a redraw of the app, waiting until that redraw has completed."""
         # If we're running slow, wait for a second
         if self.app.run_slow:
+            delay = 1
+
+        if delay:
             print("Waiting for redraw" if message is None else message)
-            await asyncio.sleep(1)
+            await asyncio.sleep(delay)

android/tests_backend/widgets/base.py

@@ -72,12 +72,12 @@ def assert_alignment(self, expected):
         else:
             assert actual == expected
 
-    async def redraw(self, message=None):
+    async def redraw(self, message=None, delay=None):
         """Request a redraw of the app, waiting until that redraw has completed."""
         self.native.requestLayout()
         await self.layout_listener.event.wait()
 
-        await super().redraw(message=message)
+        await super().redraw(message=message, delay=delay)
 
     @property
     def enabled(self):

android/tests_backend/widgets/webview.py

@@ -0,0 +1,10 @@
+from android.webkit import WebView
+
+from .base import SimpleProbe
+
+
+class WebViewProbe(SimpleProbe):
+    native_class = WebView
+    content_supports_url = False
+    javascript_supports_exception = False
+    supports_on_load = False

changes/1949.feature.rst

@@ -0,0 +1 @@
+The WebView widget now has 100% test coverage, and complete API documentation.

changes/1949.removal.1.rst

@@ -0,0 +1 @@
+The ``on_key_down`` handler has been removed from WebView. If you need to catch user input, either use a handler in the embedded Javascript, or create a ``Command`` with a key shortcut.

changes/1949.removal.2.rst

@@ -0,0 +1 @@
+The ``get_dom()`` method on WebView has been removed. This method wasn't implemented on most platforms, and wasn't working on any of the platforms where it *was* implemented, as modern web view implementations don't provide a synchronous API for accessing web content in this way.

changes/1949.removal.3.rst

@@ -0,0 +1 @@
+The ``evaluate_javascript()`` method on WebView has been modified to work in both synchronous and asynchronous contexts. In a synchronous context you can invoke the method and use a functional ``on_result`` callback to be notified when evaluation is complete. In an asynchronous context, you can await the result.

changes/1949.removal.4.rst

@@ -0,0 +1 @@
+The ``invoke_javascript()`` method has been removed. All usage of ``invoke_javascript()`` can be replaced with ``evaluate_javascript()``.

changes/1949.removal.5.rst

@@ -0,0 +1 @@
+The usage of local ``file://`` URLs has been explicitly prohibited. ``file://`` URLs have not been reliable for some time; their usage is now explicitly prohibited.

cocoa/src/toga_cocoa/widgets/webview.py

@@ -1,39 +1,46 @@
-from asyncio import get_event_loop
-from ctypes import c_void_p
-
 from rubicon.objc import objc_method, objc_property, py_from_ns
 from rubicon.objc.runtime import objc_id
 from travertino.size import at_least
 
-from ..keys import toga_key
-from ..libs import NSURL, NSURLRequest, WKWebView, send_super
+from toga.widgets.webview import JavaScriptResult
+
+from ..libs import NSURL, NSURLRequest, WKWebView
 from .base import Widget
 
 
+def js_completion_handler(future, on_result=None):
+    def _completion_handler(res: objc_id, error: objc_id) -> None:
+        if error:
+            error = py_from_ns(error)
+            exc = RuntimeError(str(error))
+            future.set_exception(exc)
+            if on_result:
+                on_result(None, exception=exc)
+        else:
+            result = py_from_ns(res)
+            future.set_result(result)
+            if on_result:
+                on_result(result)
+
+    return _completion_handler
+
+
 class TogaWebView(WKWebView):
     interface = objc_property(object, weak=True)
     impl = objc_property(object, weak=True)
 
     @objc_method
     def webView_didFinishNavigation_(self, navigation) -> None:
-        if self.interface.on_webview_load:
-            self.interface.on_webview_load(self.interface)
+        self.interface.on_webview_load(self.interface)
+
+        if self.impl.loaded_future:
+            self.impl.loaded_future.set_result(None)
+            self.impl.loaded_future = None
 
     @objc_method
     def acceptsFirstResponder(self) -> bool:
         return True
 
-    @objc_method
-    def keyDown_(self, event) -> None:
-        if self.interface.on_key_down:
-            self.interface.on_key_down(self.interface, **toga_key(event))
-        send_super(__class__, self, "keyDown:", event, argtypes=[c_void_p])
-
-    @objc_method
-    def touchBar(self):
-        # Disable the touchbar.
-        return None
-
 
 class WebView(Widget):
     def create(self):
@@ -44,77 +51,44 @@ def create(self):
         self.native.navigationDelegate = self.native
         self.native.uIDelegate = self.native
 
+        self.loaded_future = None
+
         # Add the layout constraints
         self.add_constraints()
 
-    def set_on_key_down(self, handler):
-        pass
-
-    def set_on_webview_load(self, handler):
-        pass
-
-    def get_dom(self):
-        # Utilises Step 2) of:
-        # https://developer.apple.com/library/content/documentation/
-        #       Cocoa/Conceptual/DisplayWebContent/Tasks/SaveAndLoad.html
-        html = self.native.mainframe.DOMDocument.documentElement.outerHTML
-        return html
-
     def get_url(self):
-        url = self.native.URL
-        if url:
-            return str(url)
+        url = str(self.native.URL)
+        return None if url == "about:blank" else url
 
-    def set_url(self, value):
+    def set_url(self, value, future=None):
         if value:
             request = NSURLRequest.requestWithURL(NSURL.URLWithString(value))
-            self.native.loadRequest(request)
+        else:
+            request = NSURLRequest.requestWithURL(NSURL.URLWithString("about:blank"))
+
+        self.loaded_future = future
+        self.native.loadRequest(request)
 
     def set_content(self, root_url, content):
         self.native.loadHTMLString(content, baseURL=NSURL.URLWithString(root_url))
 
+    def get_user_agent(self):
+        return str(self.native.valueForKey("userAgent"))
+
     def set_user_agent(self, value):
-        user_agent = (
-            value
-            if value
-            else (
-                "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/603.3.8 "
-                "(KHTML, like Gecko) Version/10.1.2 Safari/603.3.8"
-            )
+        self.native.customUserAgent = value
+
+    def evaluate_javascript(self, javascript: str, on_result=None) -> str:
+        result = JavaScriptResult()
+        self.native.evaluateJavaScript(
+            javascript,
+            completionHandler=js_completion_handler(
+                future=result.future,
+                on_result=on_result,
+            ),
         )
-        self.native.customUserAgent = user_agent
-
-    async def evaluate_javascript(self, javascript):
-        """Evaluate a JavaScript expression.
-
-        **This method is asynchronous**. It will return when the expression has been
-        evaluated and a result is available.
-
-        :param javascript: The javascript expression to evaluate
-        :type  javascript: ``str``
-        """
-
-        loop = get_event_loop()
-        future = loop.create_future()
-
-        def completion_handler(res: objc_id, error: objc_id) -> None:
-            if error:
-                error = py_from_ns(error)
-                exc = RuntimeError(str(error))
-                future.set_exception(exc)
-            else:
-                future.set_result(py_from_ns(res))
-
-        self.native.evaluateJavaScript(javascript, completionHandler=completion_handler)
-
-        return await future
-
-    def invoke_javascript(self, javascript):
-        """Invoke a block of javascript.
 
-        :param javascript: The javascript expression to invoke
-        """
-        self.native.evaluateJavaScript(javascript, completionHandler=None)
+        return result
 
     def rehint(self):
         self.interface.intrinsic.width = at_least(self.interface._MIN_WIDTH)

cocoa/tests_backend/probe.py

@@ -52,12 +52,15 @@ def assert_font_family(self, expected):
             SYSTEM: ".AppleSystemUIFont",
         }.get(expected, expected)
 
-    async def redraw(self, message=None):
+    async def redraw(self, message=None, delay=None):
         """Request a redraw of the app, waiting until that redraw has completed."""
         if self.app.run_slow:
             # If we're running slow, wait for a second
             print("Waiting for redraw" if message is None else message)
-            await asyncio.sleep(1)
+            delay = 1
+
+        if delay:
+            await asyncio.sleep(delay)
         else:
             # Running at "normal" speed, we need to release to the event loop
             # for at least one iteration. `runUntilDate:None` does this.