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? 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

Reply via email to