fix: removing options preferences private attributes

Author: LucasAlvws

pydoll/browser/options.py

@@ -21,12 +21,6 @@ def __init__(self):
         self._binary_location = ''
         self._start_timeout = 10
         self._browser_preferences = {}
-        self._prompt_for_download = True
-        self._block_popups = False
-        self._password_manager_enabled = True
-        self._block_notifications = False
-        self._allow_automatic_downloads = False
-        self._open_pdf_externally = False
 
     @property
     def arguments(self) -> list[str]:
@@ -131,6 +125,25 @@ def _set_pref_path(self, path: list, value):
             d = d.setdefault(key, {})
         d[path[-1]] = value
 
+    def _get_pref_path(self, path: list):
+        """
+        Safely gets a nested value from self._browser_preferences.
+
+        Arguments:
+            path -- List of keys representing the nested
+                    path (e.g., ['plugins', 'always_open_pdf_externally'])
+
+        Returns:
+            The value at the given path, or None if path doesn't exist
+        """
+        d = self._browser_preferences
+        try:
+            for key in path:
+                d = d[key]
+            return d
+        except (KeyError, TypeError):
+            return None
+
     def set_default_download_directory(self, path: str):
         """
         Set the default directory where downloaded files will be saved.
@@ -155,7 +168,7 @@ def set_accept_languages(self, languages: str):
 
     @property
     def prompt_for_download(self) -> bool:
-        return self._prompt_for_download
+        return self._get_pref_path(['download', 'prompt_for_download'])
 
     @prompt_for_download.setter
     def prompt_for_download(self, enabled: bool):
@@ -167,12 +180,11 @@ def prompt_for_download(self, enabled: bool):
         Arguments:
             enabled -- If True, Chrome will ask for confirmation before downloading.
         """
-        self._prompt_for_download = enabled
-        self._set_pref_path(['download', 'prompt_for_download'], self._prompt_for_download)
+        self._set_pref_path(['download', 'prompt_for_download'], enabled)
 
     @property
     def block_popups(self) -> bool:
-        return self._block_popups == 0
+        return self._get_pref_path(['profile', 'default_content_setting_values', 'popups']) == 0
 
     @block_popups.setter
     def block_popups(self, block: bool):
@@ -184,14 +196,13 @@ def block_popups(self, block: bool):
         Arguments:
             block -- If True, pop-ups will be blocked (value = 0); otherwise allowed (value = 1).
         """
-        self._block_popups = 0 if block else 1
         self._set_pref_path(
-            ['profile', 'default_content_setting_values', 'popups'], self._block_popups
+            ['profile', 'default_content_setting_values', 'popups'], 0 if block else 1
         )
 
     @property
     def password_manager_enabled(self) -> bool:
-        return self._password_manager_enabled
+        return self._get_pref_path(['profile', 'password_manager_enabled'])
 
     @password_manager_enabled.setter
     def password_manager_enabled(self, enabled: bool):
@@ -203,16 +214,16 @@ def password_manager_enabled(self, enabled: bool):
         Arguments:
             enabled -- If True, the password manager is active.
         """
-        self._password_manager_enabled = enabled
-        self._set_pref_path(['profile', 'password_manager_enabled'], self._password_manager_enabled)
-        self._set_pref_path(
-            ['credentials_enable_service'], self._password_manager_enabled
-        )  # todo: colocar em outra propriedade
+        self._set_pref_path(['profile', 'password_manager_enabled'], enabled)
+        self._set_pref_path(['credentials_enable_service'], enabled)
 
     @property
     def block_notifications(self) -> bool:
         block_notifications_true_value = 2
-        return self._block_notifications == block_notifications_true_value
+        return (
+            self._get_pref_path(['profile', 'default_content_setting_values', 'notifications'])
+            == block_notifications_true_value
+        )
 
     @block_notifications.setter
     def block_notifications(self, block: bool):
@@ -225,15 +236,19 @@ def block_notifications(self, block: bool):
             block -- If True, notifications will be blocked (value = 2);
             otherwise allowed (value = 1).
         """
-        self._block_notifications = 2 if block else 1
         self._set_pref_path(
             ['profile', 'default_content_setting_values', 'notifications'],
-            self._block_notifications,
+            2 if block else 1,
         )
 
     @property
     def allow_automatic_downloads(self) -> bool:
-        return self._allow_automatic_downloads == 1
+        return (
+            self._get_pref_path(
+                ['profile', 'default_content_setting_values', 'automatic_downloads']
+            )
+            == 1
+        )
 
     @allow_automatic_downloads.setter
     def allow_automatic_downloads(self, allow: bool):
@@ -246,15 +261,14 @@ def allow_automatic_downloads(self, allow: bool):
             allow -- If True, automatic downloads are allowed (value = 1);
             otherwise blocked (value = 2).
         """
-        self._allow_automatic_downloads = 1 if allow else 2
         self._set_pref_path(
             ['profile', 'default_content_setting_values', 'automatic_downloads'],
-            self._allow_automatic_downloads,
+            1 if allow else 2,
         )
 
     @property
     def open_pdf_externally(self) -> bool:
-        return self._open_pdf_externally
+        return self._get_pref_path(['plugins', 'always_open_pdf_externally'])
 
     @open_pdf_externally.setter
     def open_pdf_externally(self, enabled: bool):
@@ -266,5 +280,4 @@ def open_pdf_externally(self, enabled: bool):
         Arguments:
             block -- If True, location access is blocked (value = 2); otherwise allowed (value = 1).
         """
-        self._open_pdf_externally = enabled
-        self._set_pref_path(['plugins', 'always_open_pdf_externally'], self._open_pdf_externally)
+        self._set_pref_path(['plugins', 'always_open_pdf_externally'], enabled)

tests/test_browser/test_browser_chrome.py

@@ -516,3 +516,59 @@ async def test_chrome_user_data_dir_and_preferences(self, tmp_path):
             with open(prefs_file, 'r', encoding='utf-8') as f:
                 final_prefs = json.load(f)
             assert final_prefs == initial_prefs
+
+    @pytest.mark.asyncio
+    async def test_chrome_user_data_dir_with_invalid_json_preferences(self, tmp_path):
+        """Test Chrome with user data directory containing invalid JSON preferences."""
+        user_data_dir = tmp_path / 'chrome_profile'
+        user_data_dir.mkdir()
+        
+        prefs_dir = user_data_dir / 'Default'
+        prefs_dir.mkdir()
+        prefs_file = prefs_dir / 'Preferences'
+        
+        # Write invalid JSON to the Preferences file
+        invalid_json = '{ "profile": { "exit_type": "Normal", "exited_cleanly": true, } }' # trailing comma makes it invalid
+        prefs_file.write_text(invalid_json, encoding='utf-8')
+        
+        custom_options = ChromiumOptions()
+        custom_options.add_argument(f'--user-data-dir={user_data_dir}')
+        custom_options.browser_preferences = {
+            'test_pref': 'new_value',
+            'new_pref': 'some_value'
+        }
+        custom_options.prompt_for_download = False
+        
+        with patch.multiple(
+            Chrome,
+            _get_default_binary_location=MagicMock(return_value='/fake/chrome'),
+        ), patch(
+            'pydoll.browser.managers.browser_process_manager.BrowserProcessManager',
+            autospec=True,
+        ), patch(
+            'pydoll.browser.managers.temp_dir_manager.TempDirectoryManager',
+            autospec=True,
+        ), patch(
+            'pydoll.connection.connection_handler.ConnectionHandler',
+            autospec=True,
+        ), patch(
+            'pydoll.browser.managers.proxy_manager.ProxyManager',
+            autospec=True,
+        ):
+            async with Chrome(options=custom_options) as chrome:
+                chrome._setup_user_dir()
+                assert f'--user-data-dir={user_data_dir}' in chrome.options.arguments
+
+                # The invalid JSON should be handled gracefully by suppress(json.JSONDecodeError)
+                # and the preferences should be written with only the new preferences
+                with open(prefs_file, 'r', encoding='utf-8') as f:
+                    updated_prefs = json.load(f)
+                assert updated_prefs['test_pref'] == 'new_value'
+                assert updated_prefs['new_pref'] == 'some_value'
+                
+                # The original invalid JSON should be backed up
+                backup_file = user_data_dir / 'Default' / 'Preferences.backup'
+                assert backup_file.exists()
+                with open(backup_file, 'r', encoding='utf-8') as f:
+                    backup_content = f.read()
+                assert backup_content == invalid_json

tests/test_browser/test_browser_options.py

@@ -102,7 +102,6 @@ def test_set_open_pdf_externally():
     assert options.open_pdf_externally is True
 
 
-
 def test_set_accept_languages():
     options = Options()
     options.set_accept_languages('pt-BR,pt,en-US,en')
@@ -161,27 +160,50 @@ def test_wrong_dict_prefs_error():
             }
         }
 
+def test_set_arguments():
+    options = Options()
+    options.arguments = ['--headless']
+    assert options.arguments == ['--headless']
+
+def test_get_pref_path():
+    options = Options()
+    options.set_default_download_directory('/tmp/downloads')
+    assert options._get_pref_path(['download', 'default_directory']) == '/tmp/downloads'
+
+
+def test_get_pref_path_none():
+    options = Options()
+    assert options._get_pref_path(['download', 'default_directory']) is None
+
 
 def test_options_interface_enforcement():
     with pytest.raises(TypeError):
         OptionsInterface()
+
     class IncompleteOptions(OptionsInterface):
         pass
+
     with pytest.raises(TypeError):
         IncompleteOptions()
+
     class CompleteOptions(OptionsInterface):
         @property
         def arguments(self):
             return []
+
         @property
         def binary_location(self):
             return ''
+
         @property
         def start_timeout(self):
             return 0
+
         def add_argument(self, argument):
             pass
+
         @property
         def browser_preferences(self):
             return {}
+
     CompleteOptions()