Brock Pytlik wrote: > Padraig O'Briain wrote: >> >> >> 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? > That's one of the things it's doing, yes. But on line 302, we know > that it's not a valid URL. There are (at least) two possible reasons > for that. 1) The user isn't done typing yet, in which case we don't > want to tell them there's an error. 2) The user may still be typing, > but has entered something that can never be a valid hostname. For > example they enter http://#$$%!#$%#%.c I'd like that to be reported as > an error (right after they enter the first # ideally). Brock this is exactly what happens. The input is validated on each key stroke so:
h - invalid uri (301), but part of http:// (302) so ok no error ditto for: ht, htt, http, http:, http:/, http:// - invalid uri (301), but part of http:// (302) so ok no error http://# - invalid uri(301), and not part of http:// (302) so display the error at once and continue to do so as further invalid chars are entered. > > Does that clarify what I mean? > > Brock >> >> 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
