On 12/04/08 20:09, Brock Pytlik wrote: > jmr wrote: >> Padraig - this looks great and really improves the usability of the >> dialog. >> >> In repository.py: >> >> 301 + if not misc.valid_auth_url(name): >> 302 + if "http://".startswith(name): >> 303 + self.url_error = None >> Should you not also be allowing https:// as a valid start for a repo >> url? >> > Shouldn't line 302 also be checking that no invalid characters appear > in the string?
Is this not what line 301 is doing? I have changed the code to use _misc.valid_proto. See webrev at http://cr.opensolaris.org/~padraig/ips-5069-v4/ Padraig > The second part of line 196 in misc.py shows roughly, but probably not > exactly, what I think the test needs to include. Also, the test should > probably use _valid_proto in misc.py for the test(s) on line 302 since > it would be nice not to have to hop around the code if we decide to > support another protocol in the future. > > It's possible that the test on line 301 should be using > misc.valid_auth_prefix, at least while the user is actively typing, > but I'm not certain. > > Brock > >> JR >> >> Padraig O'Briain wrote: >> >>> The webrev http://cr.opensolaris.org/~padraig/ips-5069-v2/ fixes >>> bug 5069 Pkg Mgr GUI provides poor error feedback on Repository Add >>> >>> We now do the following: >>> >>> * Add button is enabled only when both fields have valid strings >>> * Provide character by character messaging as done now except as >>> below >>> * Remove the error graphic, but leave the text red color - the >>> message is important but the error graphic is a bit much in this >>> context >>> * Left-align the message with the text fields; alignment is very >>> awkward as it crossed the label/field divide. >>> * Do *not* display the "Name/URL is not specified" messages. The >>> user can figure out that values are required in each field. >>> * Do *not* display "URL is not valid" as long as what they are >>> typing might be valid, i.e., "http://"; is valid. >>> >>> >>> Padraig >>> ------------------------------------------------------------------------ >>> >>> >>> _______________________________________________ >>> pkg-discuss mailing list >>> [email protected] >>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss >>> >> >> _______________________________________________ >> pkg-discuss mailing list >> [email protected] >> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss >> > _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
