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.
Revised with a suggestion from @dashea to do it by having the combo box handlers emit the changed signal for their corresponding URL entry boxes, which is simpler and avoids the inadvisable practice of having different widgets use the same handler function (the repo protocol combo box no longer uses the repo URL box handler directly).
While working this out and testing it I noticed a buglet with this process:
1. Set the URL protocol to http:// 2. Type `https://www.foo.com%60 in the entry box 3. Change the URL protocol to https://
For both the base repo and additional repo implementations, after steps 1 and 2 you see a warning bar "Protocol in URL does not match selected protocol", which is good. Again, for both, after step 3, the 'https://' is stripped from the URL (so it's now just `www.foo.com`), which is also good. But for the base repo the 'protocol does not match' warning is cleared immediately - which is correct - while for the additional repo the warning remains until you make some other change.
I think this is because, somewhat oddly, the way the checks are run differs pretty significantly between the two implementations. The base URL has this `_updateURLEntryCheck()` method (which is called in several places, including `refresh()` and `_urlCheck` validator; the additional repo URL entry boxes don't have anything like that, and only get the checks run by the change handlers.
I think that buglet was there all along, though, so it shouldn't hold up this change - this fixes the crash and makes things work, I think, at least as well as they did before efa549e. I've tested it with an updates.img and fiddling around with both URL entry things quite a bit.
Looks good to me now.
Added label: ACK.
Looks good to me too, nice fix.
Closed.
anaconda-patches@lists.fedorahosted.org