New: [rhinstaller/anaconda/pulls/220 master] act on the right objects when stripping URL protocols (#1243962)
efa549ef introduced a _removeUrlPrefix() method which expected its 'editable' parameter to be a GtkEntry (either the base repo URL entry box, or an additional repo's URL entry box). However, it had the on_repoUrl_changed() handler call _removeUrlPrefix() with its own 'editable' parameter, and then had the protocol combo box call the on_repoUrl_changed() handler when it was changed, as well as the URL entry box. The intent is that if, say, you paste in 'https://www.repo.com' while the protocol is set to http://, then change the protocol to https://, the protocol will get stripped - but because 'editable' in this case is the GtkComboBoxText, not the GtkEntry, it all goes wrong and crashes. The crash is reproducible by adding a supplementary repo, entering a URL, then changing the protocol.
This adjusts things so the handler functions that call _removeUrlPrefix() don't pass in their own 'editable' params but always pass in the appropriate GtkEntry. It also makes the protocol combo box for the base repo call the appropriate handler function as well as the text entry box - matching the behaviour the original commit introduced for the additional repo widgets (I guess this was simply overlooked at the time).
I've tested this and all four widgets now behave as expected, and there are no crashes.
From: Adam Williamson awilliam@redhat.com
efa549ef introduced a _removeUrlPrefix() method which expected its 'editable' parameter to be a GtkEntry (either the base repo URL entry box, or an additional repo's URL entry box). However, it had the on_repoUrl_changed() handler call _removeUrlPrefix() with its own 'editable' parameter, and then had the protocol combo box call the on_repoUrl_changed() handler when it was changed, as well as the URL entry box. The intent is that if, say, you paste in 'https://www.repo.com' while the protocol is set to http://, then change the protocol to https://, the protocol will get stripped - but because 'editable' in this case is the GtkComboBoxText, not the GtkEntry, it all goes wrong and crashes. The crash is reproducible by adding a supplementary repo, entering a URL, then changing the protocol.
This adjusts things so the handler functions that call _removeUrlPrefix() don't pass in their own 'editable' params but always pass in the appropriate GtkEntry. It also makes the protocol combo box for the base repo call the appropriate handler function as well as the text entry box - matching the behaviour the original commit introduced for the additional repo widgets (I guess this was simply overlooked at the time).
I've tested this and all four widgets now behave as expected, and there are no crashes. --- pyanaconda/ui/gui/spokes/source.glade | 1 + pyanaconda/ui/gui/spokes/source.py | 17 ++++++++++------- 2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/source.glade b/pyanaconda/ui/gui/spokes/source.glade index 0b1045c..3d12a09 100644 --- a/pyanaconda/ui/gui/spokes/source.glade +++ b/pyanaconda/ui/gui/spokes/source.glade @@ -757,6 +757,7 @@ <item id="Closest mirror" translatable="yes">Closest mirror</item> </items> <signal name="changed" handler="on_protocol_changed" swapped="no"/> + <signal name="changed" handler="on_urlEntry_changed" swapped="no"/> </object> <packing> <property name="left_attach">0</property> diff --git a/pyanaconda/ui/gui/spokes/source.py b/pyanaconda/ui/gui/spokes/source.py index d909ec9..0cb05b8 100644 --- a/pyanaconda/ui/gui/spokes/source.py +++ b/pyanaconda/ui/gui/spokes/source.py @@ -1320,7 +1320,7 @@ def _update_repo_info(self, repo): log.error("Failed to parse proxy for repo %s: %s", repo.name, e) return
- def _removeUrlPrefix(self, editable, combo, handler): + def _removeUrlPrefix(self, url, combo, handler): # If there is a protocol in the URL, and the protocol matches the # combo box, just remove it. This makes it more convenient to paste # in URLs. It'll probably freak out people who are typing out http:// @@ -1328,7 +1328,7 @@ def _removeUrlPrefix(self, editable, combo, handler):
combo_protocol = combo.get_active_id() if combo_protocol in (PROTOCOL_HTTP, PROTOCOL_HTTPS, PROTOCOL_FTP): - url_string = editable.get_text() + url_string = url.get_text() m = URL_PARSE.match(url_string) if m: url_protocol = m.group('protocol') @@ -1336,12 +1336,14 @@ def _removeUrlPrefix(self, editable, combo, handler): (url_protocol == 'https://' and combo_protocol == PROTOCOL_HTTPS) or \ (url_protocol == 'ftp://' and combo_protocol == PROTOCOL_FTP): # URL protocol matches. Block the changed signal and remove it - with blockedHandler(editable, handler): - editable.set_text(url_string[len(url_protocol):]) + with blockedHandler(url, handler): + url.set_text(url_string[len(url_protocol):])
def on_urlEntry_changed(self, editable, data=None): # Check for and remove a URL prefix that matches the protocol dropdown - self._removeUrlPrefix(editable, self._protocolComboBox, self.on_urlEntry_changed) + # NOTE: called by different widgets, editable may be a GtkComboBoxText + # or a GtkEntry! + self._removeUrlPrefix(self._urlEntry, self._protocolComboBox, self.on_urlEntry_changed)
def on_noUpdatesCheckbox_toggled(self, *args): """ Toggle the enable state of the updates repo @@ -1415,7 +1417,8 @@ def on_repoNameEntry_changed(self, entry): self._repoChecks[name].name_check.update_check_status()
def on_repoUrl_changed(self, editable, data=None): - """ proxy url or protocol changed + """ proxy url or protocol changed. NOTE: called by different + objects, editable may be a GtkComboBoxText or a GtkEntry! """ itr = self._repoSelection.get_selected()[1] if not itr: @@ -1432,7 +1435,7 @@ def on_repoUrl_changed(self, editable, data=None): self._repoChecks[repo.name].url_check.update_check_status()
# Check for and remove a URL prefix that matches the protocol dropdown - self._removeUrlPrefix(editable, self._repoProtocolComboBox, self.on_repoUrl_changed) + self._removeUrlPrefix(self._repoUrlEntry, self._repoProtocolComboBox, self.on_repoUrl_changed)
def on_repoMirrorlistCheckbox_toggled(self, *args): """ mirror state changed
anaconda-patches@lists.fedorahosted.org
-
dashea