Amend Bookmark methods

foxpuppet/windows/browser/bookmarks/bookmark.py

@@ -8,7 +8,7 @@
 from selenium.webdriver.remote.webelement import WebElement
 from selenium.webdriver.common.keys import Keys
 from foxpuppet.windows.browser.navbar import NavBar
-from typing import Type, Any, TYPE_CHECKING, Optional, TypedDict, List
+from typing import TYPE_CHECKING, Optional, TypedDict, List
 
 
 class BookmarkData(TypedDict):
@@ -32,7 +32,7 @@ def create(window: "BrowserWindow", root: WebElement) -> Optional["BasicBookmark
 
         Args:
             window (:py:class:`BrowserWindow`): Window object this bookmark appears in
-            root (:py:class:`~selenium.webdriver.remote.webelement.WebElement`): WebDriver element object for the bookmark panel
+            root (:py:class:`~selenium.webdriver.remote.webelement.WebElement`): WebDriver element object for bookmark
 
         Returns:
             :py:class:`BaseBookmark`: Bookmark instance or None
@@ -52,10 +52,7 @@ def is_bookmarked(self) -> bool:
         """
         with self.selenium.context(self.selenium.CONTEXT_CHROME):
             star_button_image = self.find_element(BookmarkLocators.STAR_BUTTON_IMAGE)
-            if star_button_image is not None:
-                return star_button_image.get_attribute("starred") == "true"
-            else:
-                return False
+            return star_button_image.get_attribute("starred") == "true"
 
     def add(self) -> None:
         """Add a Bookmark using the star button."""
@@ -66,7 +63,7 @@ def add(self) -> None:
 
     def retrieve_bookmark(self, label: str) -> bool:
         """
-        Check if a bookmark with the given label exists under 'Other Bookmarks'.
+        Check if a bookmark with the given label exists.
 
         Args:
             label (str): The name of the bookmark to search for.
@@ -77,18 +74,15 @@ def retrieve_bookmark(self, label: str) -> bool:
                 locator_bookmark=BookmarkLocators.PANEL_BOOKMARK_MENU,
             )
             panel_bookmarks = self.find_element(BookmarkLocators.PANEL_BOOKMARK_TOOLBAR)
-            if panel_bookmarks is None:
-                return False
-            menu_items = panel_bookmarks.find_elements(
-                By.CSS_SELECTOR, "toolbarbutton.bookmark-item"
-            )
-            if not menu_items:
-                return False
-            for item in menu_items:
-                item_label = item.get_attribute("label")
-                if item_label and label.lower() in item_label.lower():
-                    return True
-
+            if panel_bookmarks is not None:
+                menu_items = panel_bookmarks.find_elements(
+                    By.CSS_SELECTOR, "toolbarbutton.bookmark-item"
+                )
+                if menu_items is not None:
+                    for item in menu_items:
+                        item_label = item.get_attribute("label")
+                        if item_label and label.lower() in item_label.lower():
+                            return True
             return False
 
     def delete(self) -> None:
@@ -136,19 +130,17 @@ def is_bookmarked(self) -> bool:
             locator_menu_bar=BookmarkLocators.MENU_BAR,
         )
         bookmark_menu = self.find_element(BookmarkLocators.MAIN_MENU_BOOKMARK)
-        if bookmark_menu is None:
-            return False
-        self.click_element(BookmarkLocators.MAIN_MENU_BOOKMARK)
-        with self.selenium.context(self.selenium.CONTEXT_CHROME):
-            menu_items = bookmark_menu.find_elements(
-                By.CSS_SELECTOR, "menuitem.bookmark-item"
-            )
-            for item in menu_items:
-                item_label = item.get_attribute("label")
-                if item_label and item_label.lower() in current_page_title.lower():
-                    return True
-
-            return False
+        if bookmark_menu is not None:
+            self.click_element(BookmarkLocators.MAIN_MENU_BOOKMARK)
+            with self.selenium.context(self.selenium.CONTEXT_CHROME):
+                menu_items = bookmark_menu.find_elements(
+                    By.CSS_SELECTOR, "menuitem.bookmark-item"
+                )
+                for item in menu_items:
+                    item_label = item.get_attribute("label")
+                    if item_label and item_label.lower() in current_page_title.lower():
+                        return True
+        return False
 
     def add_bookmark(self, bookmark_data: BookmarkData) -> None:
         """Add a Bookmark using the main bookmark menu."""
@@ -185,23 +177,21 @@ def add_bookmark(self, bookmark_data: BookmarkData) -> None:
         self.actions.send_keys(Keys.TAB, Keys.ENTER).perform()
         self.switch_to_default_context()
 
-    def delete_bookmark(self, label: str) -> None:
+    def delete_bookmark(self, label: str) -> bool:
         """Delete a bookmark using the main bookmark menu."""
         self.open_main_menu(
             locator_toolbar=BookmarkLocators.NAVIGATOR_TOOLBOX,
             locator_menu_bar=BookmarkLocators.MENU_BAR,
         )
         bookmark_menu = self.find_element(BookmarkLocators.MAIN_MENU_BOOKMARK)
-        if bookmark_menu is None:
-            raise ValueError("Bookmark menu not found")
         self.click_element(BookmarkLocators.MAIN_MENU_BOOKMARK)
         with self.selenium.context(self.selenium.CONTEXT_CHROME):
             menu_item = bookmark_menu.find_element(
                 By.CSS_SELECTOR, f"menuitem.bookmark-item[label='{label}']"
             )
             self.actions.context_click(menu_item).perform()
             self.click_element(BookmarkLocators.DELETE_MENU_ITEM)
-
+        return True
 
 class BookmarkLocators:
     ADD_BOOKMARK = (By.ID, "placesContext_new:bookmark")

foxpuppet/windows/browser/navbar.py

@@ -52,21 +52,18 @@ def is_tracking_shield_displayed(self) -> bool:
     def click_element(self, locator: Tuple[str, str]) -> None:
         """Click on an element by its locator."""
         with self.selenium.context(self.selenium.CONTEXT_CHROME):
-            element = self.find_element(locator)
-            if element is not None:
-                element.click()
-            else:
-                raise NoSuchElementException(f"Element with locator {locator} not found.")
+            self.find_element(locator).click()
 
-    def find_element(self, locator: Tuple[str, str]) -> Optional[WebElement]:
+    def find_element(self, locator: Tuple[str, str]) -> WebElement:
         """Find and return a web element by its locator."""
         with self.selenium.context(self.selenium.CONTEXT_CHROME):
             try:
                 element = self.root.find_element(*locator)
                 return element
             except Exception as e:
-                raise NoSuchElementException(f"Error locating element with locator {locator}: {e}")
-                return None
+                raise NoSuchElementException(
+                    f"Error locating element with locator {locator}: {e}"
+                )
 
     def context_click(self, locator: Tuple[str, str]) -> None:
         """

foxpuppet/windows/browser/notifications/addons.py

@@ -57,17 +57,38 @@ def close(self) -> None:
                 BaseNotification.close(self)
 
 
-class AddOnInstallRestart(BaseNotification):
-    """Add-on install restart notification."""
-
-
 class AddOnInstallFailed(BaseNotification):
     """Add-on install failed notification."""
 
+    @property
+    def error_message(self):
+        """Provide access to the error message.
+
+        Returns:
+            str: The error message explaining why the installation failed.
+        """
+        with self.selenium.context(self.selenium.CONTEXT_CHROME):
+            return self.find_description().text
+
+    def close(self):
+        """Close the failed installation notification."""
+        with self.selenium.context(self.selenium.CONTEXT_CHROME):
+            self.find_primary_button().click()
+
 
 class AddOnProgress(BaseNotification):
     """Add-on progress notification."""
 
+    @property
+    def is_downloading(self):
+        """Check if the add-on is currently downloading.
+
+        Returns:
+            bool: True if the download and verification is in progress.
+        """
+        with self.selenium.context(self.selenium.CONTEXT_CHROME):
+            return "Downloading and verifying add-on…" in self.find_description().text
+
 
 # Clean up of these notifications will happen once Firefox ESR is past version 63
 # https://github.com/mozilla/FoxPuppet/issues/212
@@ -76,7 +97,6 @@ class AddOnProgress(BaseNotification):
     "addon-install-confirmation-notification": AddOnInstallConfirmation,
     "addon-install-complete-notification": AddOnInstallComplete,
     "appMenu-addon-installed-notification": AddOnInstallComplete,
-    "addon-install-restart-notification": AddOnInstallRestart,
     "addon-install-failed-notification": AddOnInstallFailed,
     "addon-installed-notification": AddOnInstallComplete,
     "addon-progress-notification": AddOnProgress,

foxpuppet/windows/browser/window.py

@@ -13,7 +13,9 @@
 from foxpuppet.windows.browser.bookmarks.bookmark import BasicBookmark
 from foxpuppet.windows.browser.bookmarks.bookmark import AdvancedBookmark
 from selenium.webdriver.remote.webelement import WebElement
-from typing import Any, Optional, Union, Type
+from typing import Any, Optional, Union, TypeVar, Type
+
+T = TypeVar("T", bound="BaseNotification")
 
 
 class BrowserWindow(BaseWindow):
@@ -98,8 +100,9 @@ def advanced_bookmark(self) -> Optional[AdvancedBookmark]:
                 return None
 
     def wait_for_notification(
-        self, notification_class: Optional[type["BaseNotification"]] = BaseNotification
-    ) -> BaseNotification | Any:
+        self,
+        notification_class: Optional[Type[T]] = BaseNotification,  # type: ignore
+    ) -> Optional[T]:
         """Wait for the specified notification to be displayed.
 
         Args:
@@ -109,7 +112,7 @@ def wait_for_notification(
                 `BaseNotification`.
 
         Returns:
-            :py:class:`BaseNotification`: Firefox notification.
+            Optional[:py:class:`BaseNotification`]: Firefox notification or None.
 
         """
         if notification_class:
@@ -121,13 +124,13 @@ def wait_for_notification(
                 lambda _: isinstance(self.notification, notification_class),
                 message=message,
             )
-            return self.notification
+            return self.notification  # type: ignore
         else:
             self.wait.until(
                 lambda _: self.notification is None,
                 message="Unexpected notification shown.",
             )
-        return None
+            return None
 
     def wait_for_bookmark(
         self,

tests/test_bookmarks.py

@@ -164,14 +164,14 @@ def test_retrieve_advanced_bookmark(advanced_bookmark: AdvancedBookmark) -> None
     assert advanced_bookmark.retrieve_bookmark(label) is True
 
 
+def test_retrieve_deleted_bookmark(basic_bookmark: BasicBookmark) -> None:
+    """Test retrieve deleted bookmark."""
+    basic_bookmark.delete()
+    assert basic_bookmark.retrieve_bookmark("any label") is False
+
+
 def test_delete_advanced_bookmark(advanced_bookmark: AdvancedBookmark) -> None:
     """Delete Advanced bookmark"""
     label = "Internet for people"
     advanced_bookmark.delete_bookmark(label)
     assert advanced_bookmark.is_bookmarked is False
-
-
-def test_retrieve_deleted_bookmark(basic_bookmark: BasicBookmark) -> None:
-    """Test retrieve deleted bookmark."""
-    basic_bookmark.delete()
-    assert basic_bookmark.retrieve_bookmark("any label") is False