Update Add-on Store mirror to use new UX

source/gui/_SetURLDialog.py

@@ -20,6 +20,12 @@
 from .settingsDialogs import SettingsDialog
 
 
+class _ValidationError(ValueError):
+	"""Exception raised when validation of an OK response returns False.
+	This is only used internally.
+	"""
+
+
 class _SetURLDialog(SettingsDialog):
 	class _URLTestStatus(Enum):
 		UNTESTED = auto()
@@ -36,6 +42,7 @@ def __init__(
 		configPath: Iterable[str],
 		helpId: str | None = None,
 		urlTransformer: Callable[[str], str] = lambda url: url,
+		responseValidator: Callable[[requests.Response], bool] = lambda response: True,
 		*args,
 		**kwargs,
 	):
@@ -46,6 +53,8 @@ def __init__(
 		:param configPath: Where in the config the URL is to be stored.
 		:param helpId: Anchor of the user guide section for this dialog, defaults to None
 		:param urlTransformer: Function to transform the given URL into something usable, eg by adding required query parameters. Defaults to the identity function.
+		:param responseValidator: Function to check that the response returned when querying the transformed URL is valid.
+			The response will always have a status of 200 (OK). Defaults to always returning True.
 		:raises ValueError: If no config path is given.
 		"""
 		if not configPath or len(configPath) < 1:
@@ -54,6 +63,7 @@ def __init__(
 		self.helpId = helpId
 		self._configPath = configPath
 		self._urlTransformer = urlTransformer
+		self._responseValidator = responseValidator
 		super().__init__(parent, *args, **kwargs)
 
 	def makeSettings(self, settingsSizer: wx.Sizer):
@@ -65,13 +75,13 @@ def makeSettings(self, settingsSizer: wx.Sizer):
 			wx.TextCtrl,
 			size=(250, -1),
 		)
-		self.bindHelpEvent("UpdateMirrorURL", urlControl)
+		self.bindHelpEvent("SetURLTextbox", urlControl)
 		self._testButton = testButton = wx.Button(
 			self,
 			# Translators: A button in a dialog which allows the user to test a URL that they have entered.
 			label=_("&Test..."),
 		)
-		self.bindHelpEvent("UpdateMirrorTest", testButton)
+		self.bindHelpEvent("SetURLTest", testButton)
 		urlControlsSizerHelper = guiHelper.BoxSizerHelper(self, sizer=urlControl.GetContainingSizer())
 		urlControlsSizerHelper.addItem(testButton)
 		testButton.Bind(wx.EVT_BUTTON, self._onTest)
@@ -147,9 +157,11 @@ def _bg(self):
 		try:
 			with requests.get(self._urlTransformer(self._url)) as r:
 				r.raise_for_status()
+				if not self._responseValidator(r):
+					raise _ValidationError
 			self._success()
-		except RequestException as e:
-			log.debug(f"Failed to check URL: {e}")
+		except (RequestException, _ValidationError) as e:
+			log.debug(f"URL check failed: {e}")
 			self._failure(e)
 
 	def _success(self):
@@ -168,15 +180,32 @@ def _success(self):
 
 	def _failure(self, error: Exception):
 		"""Notify the user that testing their URL failed."""
+		if isinstance(error, _ValidationError):
+			message = _(
+				# Translators: Message shown to users when testing a given URL has failed because the response was invalid.
+				"The URL you have entered failed the connection test. The response received from the server was invalid. Check the URL is correct before trying again.",
+			)
+		elif isinstance(error, requests.HTTPError):
+			message = _(
+				# Translators: Message shown to users when testing a given URL has failed because the server returned an error.
+				"The URL you have entered failed the connection test. The server returned an error. Check that the URL is correct before trying again.",
+			)
+		elif isinstance(error, requests.ConnectionError):
+			message = _(
+				# Translators: Message shown to users when testing a given URL has failed because of a network error.
+				"The URL you have entered failed the connection test. There was a network error. Check that you are connected to the internet and try again.",
+			)
+		else:
+			message = _(
+				# Translators: Message shown to users when testing a given URL has failed for unknown reasons.
+				"The URL you have entered failed the connection test. Make sure you are connected to the internet and the URL is correct.",
+			)
 		wx.CallAfter(self._progressDialog.done)
 		self._progressDialog = None
 		self._testStatus = _SetURLDialog._URLTestStatus.FAILED
 		wx.CallAfter(
 			gui.messageBox,
-			_(
-				# Translators: Message displayed to users when testing a URL has failed.
-				"Unable to connect to the given URL. Check that you are connected to the internet and the URL is correct.",
-			),
+			message,
 			# Translators: The title of a dialog presented when an error occurs.
 			"Error",
 			wx.OK | wx.ICON_ERROR,

source/gui/settingsDialogs.py

@@ -15,6 +15,7 @@
 from locale import strxfrm
 import re
 import typing
+import requests
 import wx
 from NVDAState import WritePaths
 
@@ -62,7 +63,6 @@
 	Set,
 	cast,
 )
-from url_normalize import url_normalize
 import core
 import keyboardHandler
 import characterProcessing
@@ -996,7 +996,7 @@ def onChangeMirrorURL(self, evt: wx.CommandEvent | wx.KeyEvent):
 			# Translators: Title of the dialog used to change NVDA's update server mirror URL.
 			title=_("Set NVDA Update Mirror"),
 			configPath=("update", "serverURL"),
-			helpId="SetUpdateMirror",
+			helpId="SetURLDialog",
 			urlTransformer=lambda url: f"{url}?versionType=stable",
 		)
 		ret = changeMirror.ShowModal()
@@ -3274,27 +3274,83 @@ def makeSettings(self, settingsSizer: wx.BoxSizer) -> None:
 		index = [x.value for x in AddonsAutomaticUpdate].index(config.conf["addonStore"]["automaticUpdates"])
 		self.automaticUpdatesComboBox.SetSelection(index)
 
-		# Translators: This is the label for a text box in the add-on store settings dialog.
-		self.addonMetadataMirrorLabelText = _("Server &mirror URL")
-		self.addonMetadataMirrorTextbox = sHelper.addLabeledControl(
-			self.addonMetadataMirrorLabelText,
-			wx.TextCtrl,
+		# Translators: The label for the server  mirror on the Add-on store Settings panel.
+		mirrorBoxSizer = wx.StaticBoxSizer(wx.HORIZONTAL, self, label=_("Server mirror"))
+		mirrorBox = mirrorBoxSizer.GetStaticBox()
+		mirrorBoxSizerHelper = guiHelper.BoxSizerHelper(self, sizer=mirrorBoxSizer)
+		sHelper.addItem(mirrorBoxSizerHelper)
+
+		# Use an ExpandoTextCtrl because even when read-only it accepts focus from keyboard, which
+		# standard read-only TextCtrl does not. ExpandoTextCtrl is a TE_MULTILINE control, however
+		# by default it renders as a single line. Standard TextCtrl with TE_MULTILINE has two lines,
+		# and a vertical scroll bar. This is not neccessary for the single line of text we wish to
+		# display here.
+		# Note: To avoid code duplication, the value of this text box will be set in `onPanelActivated`.
+		self.mirrorURLTextBox = ExpandoTextCtrl(
+			mirrorBox,
+			size=(self.scaleSize(250), -1),
+			style=wx.TE_READONLY,
 		)
-		self.addonMetadataMirrorTextbox.SetValue(config.conf["addonStore"]["baseServerURL"])
-		self.bindHelpEvent("AddonStoreMetadataMirror", self.addonMetadataMirrorTextbox)
+		# Translators: This is the label for the button used to change the Add-on Store mirror URL,
+		# it appears in the context of the Server mirror group on the Add-on Store page of NVDA's settings.
+		changeMirrorBtn = wx.Button(mirrorBox, label=_("Change..."))
+		mirrorBoxSizerHelper.addItem(
+			guiHelper.associateElements(
+				self.mirrorURLTextBox,
+				changeMirrorBtn,
+			),
+		)
+		self.bindHelpEvent("AddonStoreMetadataMirror", mirrorBox)
+		self.mirrorURLTextBox.Bind(wx.EVT_CHAR_HOOK, self._enterTriggersOnChangeMirrorURL)
+		changeMirrorBtn.Bind(wx.EVT_BUTTON, self.onChangeMirrorURL)
 
-	def isValid(self) -> bool:
-		self.addonMetadataMirrorTextbox.SetValue(
-			url_normalize(self.addonMetadataMirrorTextbox.GetValue().strip()).rstrip("/"),
+	def onChangeMirrorURL(self, evt: wx.CommandEvent | wx.KeyEvent):
+		"""Show the dialog to change the Add-on Store mirror URL, and refresh the dialog in response to the URL being changed."""
+		# Import late to avoid circular dependency.
+		from gui._SetURLDialog import _SetURLDialog
+
+		changeMirror = _SetURLDialog(
+			self,
+			# Translators: Title of the dialog used to change the Add-on Store server mirror URL.
+			title=_("Set Add-on Store Server Mirror"),
+			configPath=("addonStore", "baseServerURL"),
+			helpId="SetURLDialog",
+			urlTransformer=lambda url: f"{url}/cacheHash.json",
+			responseValidator=_isResponseAddonStoreCacheHash,
 		)
-		return True
+		ret = changeMirror.ShowModal()
+		if ret == wx.ID_OK:
+			self.Freeze()
+			# trigger a refresh of the settings
+			self.onPanelActivated()
+			self._sendLayoutUpdatedEvent()
+			self.Thaw()
+
+	def _enterTriggersOnChangeMirrorURL(self, evt: wx.KeyEvent):
+		"""Open the change update mirror URL dialog in response to the enter key in the mirror URL read-only text box."""
+		if evt.KeyCode == wx.WXK_RETURN:
+			self.onChangeMirrorURL(evt)
+		else:
+			evt.Skip()
+
+	def _updateCurrentMirrorURL(self):
+		self.mirrorURLTextBox.SetValue(
+			(
+				url
+				if (url := config.conf["addonStore"]["baseServerURL"])
+				# Translators: A value that appears in NVDA's Settings to indicate that no mirror is in use.
+				else _("No mirror")
+			),
+		)
+
+	def onPanelActivated(self):
+		self._updateCurrentMirrorURL()
+		super().onPanelActivated()
 
 	def onSave(self):
 		index = self.automaticUpdatesComboBox.GetSelection()
 		config.conf["addonStore"]["automaticUpdates"] = [x.value for x in AddonsAutomaticUpdate][index]
 
-		config.conf["addonStore"]["baseServerURL"] = self.addonMetadataMirrorTextbox.Value.strip().rstrip("/")
-
 
 class TouchInteractionPanel(SettingsPanel):
 	# Translators: This is the label for the touch interaction settings panel.
@@ -5520,3 +5576,14 @@ def onFilterEditTextChange(self, evt):
 		self.filter(self.filterEdit.Value)
 		self._refreshVisibleItems()
 		evt.Skip()
+
+
+def _isResponseAddonStoreCacheHash(response: requests.Response) -> bool:
+	try:
+		# Attempt to parse the response as JSON
+		data = response.json()
+	except ValueError:
+		# Add-on Store cache hash is JSON, so this can't be it.
+		return False
+	# Add-on Store cache hash is a git commit hash as a string.
+	return isinstance(data, str) and bool(re.fullmatch("[0-9a-fA-F]{7,40}", data))

user_docs/en/userGuide.md

@@ -1839,30 +1839,11 @@ This may be of use in locations where access to the NV Access NVDA update server
 The read-only text box shows the current mirror URL.
 If no mirror is in use (I.E. the NV Access NVDA update server is being used), "No mirror" is displayed.
 
-If you wish to change the update mirror, press the "Change..." button to open the [Set Update Mirror dialog](#SetUpdateMirror).
-
-#### Set Update Mirror {#SetUpdateMirror}
-
-This dialog, which is accessed from the [Update mirror section of the General page in NVDA's Settings dialog](#UpdateMirror), allows you to set a mirror to use when checking for NVDA updates.
-This may be of use in locations where access to the NV Access NVDA update server is slow or unavailable.
+If you wish to change the update mirror, press the "Change..." button to open the [Set Update Mirror dialog](#SetURLDialog).
 
 Please note that when using an update mirror, the operator of the mirror has access to all [information sent with update checks](#GeneralSettingsCheckForUpdates).
 Contact the operator of the update mirror for details of their data handling policies to ensure you are comfortable with the way your information will be handled before setting an update mirror.
 
-##### URL {#UpdateMirrorURL}
-
-Enter the URL (web address) of the update server mirror you wish to use here.
-Only HTTP and HTTPS URLs are supported.
-For your privacy, NV Access recommends using HTTPS URLs whenever possible.
-
-Leave this blank to use the NV Access NVDA update check server.
-
-##### Test... {#UpdateMirrorTest}
-
-Press this button to test the NVDA update server URL you have entered.
-You must be connected to the internet for the test to succeed.
-It is recommended that you always test the URL before saving it.
-
 #### Speech Settings {#SpeechSettings}
 
 <!-- KC:setting -->
@@ -3107,12 +3088,15 @@ For example, for installed beta add-ons, you will only be notified of updates wi
 |Notify |Notify when updates are available to add-ons within the same channel |
 |Disabled |Do not automatically check for updates to add-ons |
 
-##### Server mirror URL {#AddonStoreMetadataMirror}
+##### Server mirror {#AddonStoreMetadataMirror}
 
-This option allows you to specify an alternative URL to download Add-on Store data from.
+These controls allow you to specify an alternative URL to download Add-on Store data from.
 This may be of use in locations where access to the NV Access Add-on Store server is slow or unavailable.
 
-Leave this blank to use the default NV Access Add-on Store server.
+The read-only text box shows the current mirror URL.
+If no mirror is in use (I.E. the NV Access NVDA Add-on Store server is being used), "No mirror" is displayed.
+
+If you wish to change the Add-on Store server mirror, press the "Change..." button to open the [Set Add-on Store Mirror dialog](#SetURLDialog).
 
 #### Windows OCR Settings {#Win10OcrSettings}
 
@@ -3131,6 +3115,28 @@ This can be very useful when you want to monitor constantly changing content, su
 The refresh takes place every one and a half seconds.
 This option is disabled by default.
 
+#### Set Mirror Dialog {#SetURLDialog}
+
+This dialog allows you to specify the URL of a mirror to use when [updating NVDA](#GeneralSettingsCheckForUpdates) or [using the Add-on Store](#AddonsManager).
+This may be of use in locations where access to the NV Access servers for these functions is slow or unavailable.
+
+* When setting the [NVDA update mirror](#UpdateMirror), the title of this dialog will be "Set NVDA Update Mirror".
+* When setting the [Add-on Store server mirror](#AddonStoreMetadataMirror), the title of this dialog will be "Set Add-on Store Server Mirror".
+
+##### URL {#SetURLTextbox}
+
+Enter the URL (web address) of the mirror you wish to use here.
+Only HTTP and HTTPS URLs are supported.
+For your privacy, NV Access recommends using HTTPS URLs whenever possible.
+
+Leave this blank to use the default NV Access server.
+
+##### Test... {#SetURLTest}
+
+Press this button to test the mirror URL you have entered.
+You must be connected to the internet for the test to succeed.
+It is recommended that you always test the URL before saving it.
+
 #### Advanced Settings {#AdvancedSettings}
 
 Warning! The settings in this category are for advanced users and may cause NVDA to not function correctly if configured in the wrong way.