[widget audit] toga.Table

android/src/toga_android/widgets/table.py

@@ -18,7 +18,7 @@
 from .base import Widget
 
 
-class TogaOnClickListener(OnClickListener):
+class TogaOnClickListener(OnClickListener):  # pragma: no cover
     def __init__(self, impl):
         super().__init__()
         self.impl = impl
@@ -41,7 +41,7 @@ def onClick(self, view):
             self.impl.interface.on_select(self.impl.interface, row=row)
 
 
-class Table(Widget):
+class Table(Widget):  # pragma: no cover
     table_layout = None
     color_selected = None
     color_unselected = None
@@ -184,7 +184,7 @@ def get_selection(self):
         selection = []
         for row_index in range(len(self.interface.data)):
             if row_index in self.selection:
-                selection.append(self.selection[row_index])
+                selection.append(row_index)
         if len(selection) == 0:
             selection = None
         elif not self.interface.multiple_select:
@@ -209,13 +209,7 @@ def remove(self, item, index):
     def scroll_to_row(self, row):
         pass
 
-    def set_on_select(self, handler):
-        pass
-
-    def set_on_double_click(self, handler):
-        self.interface.factory.not_implemented("Table.set_on_double_click()")
-
-    def add_column(self, heading, accessor):
+    def insert_column(self, index, heading, accessor):
         self.change_source(self.interface.data)
 
     def remove_column(self, accessor):

android/tests_backend/icons.py

@@ -0,0 +1,32 @@
+import pytest
+
+from .probe import BaseProbe
+
+
+class IconProbe(BaseProbe):
+    # Android only supports 1 format, so the alternate is the same as the primary.
+    alternate_resource = "resources/icons/blue"
+
+    def __init__(self, app, icon):
+        super().__init__()
+        self.app = app
+        self.icon = icon
+        # At least for now, there's no native object.
+        # assert isinstance(self.icon._impl.native, NSImage)
+
+    def assert_icon_content(self, path):
+        if path == "resources/icons/green":
+            assert (
+                self.icon._impl.path
+                == self.app.paths.app / "resources" / "icons" / "green.png"
+            )
+        elif path == "resources/icons/blue":
+            assert (
+                self.icon._impl.path
+                == self.app.paths.app / "resources" / "icons" / "blue.png"
+            )
+        else:
+            pytest.fail("Unknown icon resource")
+
+    def assert_default_icon_content(self):
+        assert self.icon._impl.path == self.app.paths.toga / "resources" / "toga.png"

changes/2011.feature.1.rst

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

changes/2011.feature.2.rst

@@ -0,0 +1 @@
+Tables can now omit the header row by specifying ``headings=None`` and providing accessors.

changes/2011.removal.1.rst

@@ -0,0 +1 @@
+``Table.on_double_click`` has been renamed ``Table.on_activate``.

changes/2011.removal.2.rst

@@ -0,0 +1 @@
+Tables now use an empty string for the default missing value on a Table.

changes/2011.removal.3.rst

@@ -0,0 +1 @@
+``Table.add_column()`` has been deprecated in favor of ``Table.append_column()`` and ``Table.insert_column()``

cocoa/src/toga_cocoa/icons.py

@@ -9,5 +9,26 @@ def __init__(self, interface, path):
         self.interface = interface
         self.interface._impl = self
         self.path = path
+        try:
+            # We *should* be able to do a direct NSImage.alloc.init...(), but if the
+            # image file is invalid, the init fails, and returns NULL - but we've
+            # created an ObjC instance, so when the object passes out of scope, Rubicon
+            # tries to free it, which segfaults. To avoid this, we retain result of the
+            # alloc() (overriding the default Rubicon behavior of alloc), then release
+            # that reference once we're done. If the image was created successfully, we
+            # temporarily have a reference count that is 1 higher than it needs to be;
+            # if it fails, we don't end up with a stray release.
+            image = NSImage.alloc().retain()
+            self.native = image.initWithContentsOfFile(str(path))
+            if self.native is None:
+                raise ValueError(f"Unable to load icon from {path}")
+        finally:
+            image.release()
 
-        self.native = NSImage.alloc().initWithContentsOfFile(str(path))
+        # Multiple icon interface instances can end up referencing the same native
+        # instance, so make sure we retain a reference count at the impl level.
+        self.native.retain()
+
+    def __del__(self):
+        if self.native:
+            self.native.release()

cocoa/src/toga_cocoa/images.py

@@ -13,11 +13,14 @@ def __init__(self, interface, path=None, data=None):
         self.interface = interface
 
         try:
-            # We *should* be able to do a direct NSImage.alloc.init...(),
-            # but for some reason, this segfaults in some environments
-            # when loading invalid images. On iOS we can avoid this by
-            # using the class-level constructors; on macOS we need to
-            # ensure we have a valid allocated image, then try to init it.
+            # We *should* be able to do a direct NSImage.alloc.init...(), but if the
+            # image file is invalid, the init fails, and returns NULL - but we've
+            # created an ObjC instance, so when the object passes out of scope, Rubicon
+            # tries to free it, which segfaults. To avoid this, we retain result of the
+            # alloc() (overriding the default Rubicon behavior of alloc), then release
+            # that reference once we're done. If the image was created successfully, we
+            # temporarily have a reference count that is 1 higher than it needs to be;
+            # if it fails, we don't end up with a stray release.
             image = NSImage.alloc().retain()
             if path:
                 self.native = image.initWithContentsOfFile(str(path))

cocoa/src/toga_cocoa/widgets/internal/cells.py

@@ -120,7 +120,9 @@ def setup(self):
 
     @objc_method
     def setImage_(self, image):
-        if not self.imageView:
+        # This branch is here for future protection - but the image is *never* set
+        # before the text, so it can't ever happen.
+        if not self.imageView:  # pragma: no cover
             self.setup()
 
         if image: