802860 - input type url validates too strictly at expense of usability

Status: NEW

(Core :: DOM: Core & HTML, defect, P5)

Description

[Mikey Clarke]

Attachment: Screen Shot 2012-10-18 at 00.02.52.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_2) AppleWebKit/537.15+ (KHTML, like Gecko) Version/6.0.1 Safari/536.26.14

Steps to reproduce:

Tried to submit a form with a URL input into an <input type="url"> in the format www.site.com.

This issue can easily be replicated using this page: http://wufoo.com/html5/types/3-url.html


Actual results:

Firefox refused to submit the form and gave a 'Please enter a URL' notification.


Expected results:

The URL should have been accepted and the form submitted.

To an ordinary user, a website URL is 'www.somewebsite.com', not 'http://www.somewebsite.com'. Requiring the protocol to pass validation, and, further, not even alerting the user that the absence of a protocol is the reason for the URL not validating is not at all user-friendly.

Even I would be unlikely to submit a URL with the protocol, though, I'd at least know to do so as soon as it failed validation. As a developer, I have often thought twice about using type="url" due to this behaviour, and currently use the novalidate attribute on any such input to protect my users from this.

If as a developer I needed the protocol to be included I would either, a) if possible attach it myself and save the user the trouble, or b) clearly notify the user that it is required. A further issue is that in some cases developers may need the URL without the protocol.

Such strict validation defeats the object of having validation in the first place; this behaviour is extremely hostile to the average user who should not be expected to know to use protocols in URLs.

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

This is defined in the following places.

http://www.whatwg.org/specs/web-apps/current-work/multipage/states-of-the-type-attribute.html#url-state-%28type=url%29

http://www.whatwg.org/specs/web-apps/current-work/multipage/urls.html#absolute-url

Comment 2

[Mikey Clarke]

On the WHATWG mailing list (http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2012-October/037629.html):

The spec expects browsers to convert "www.example.com" to 
"http://www.example.com/" automatically so that this kind of issue 
doesn't occur.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Mounir?

Comment 4

[Mounir Lamouri (:mounir)]

I agree. However, we still need to send to the server "http://www.foo.com" even if the user typed "www.foo.com". That means we should be careful UI wise (we might end-up with false-positive) and we need to be able to handle this with the <input>. Whether by showing something (www.foo.com) different from the .value (http://www.foo.com) or by sending something (http://www.foo.com) different from the .value (www.foo.com).

Comment 5

[Mikey Clarke]

Opera correctly implements this. The way they do it, on blur 'www.example.com' becomes 'http://www.example.com', so the change is visible to the user.

Comment 6

[Boris Zbarsky [:bzbarsky]]

I would think the spec would define what .value is, no?

Mikey, that's not obviously sufficient because a submit can happen without blurring the input, no?

Comment 7

[Mikey Clarke]

That's true. Inserting 'http://' on focus would be a better way to do this, but it's still hardly elegant. I'm not convinced of the need to show the user the protocol at all. If a user neglects to provide a protocol it seems likely that they intended 'http'. It seems very unlikely that someone wishing to use 'https' would not provide it since 'http' is the usual default. Alas, Ian Hickson's message suggests that the spec disagrees with me.

Comment 8

[Mounir Lamouri (:mounir)]

Specs say:
"""
If the element is mutable, the user agent should allow the user to change the URL represented by its value. User agents may allow the user to set the value to a string that is not a valid absolute URL, but may also or instead automatically escape characters entered by the user so that the value is always a valid absolute URL (even if that isn't the actual value seen and edited by the user in the interface). User agents should allow the user to set the value to the empty string. User agents must not allow users to insert U+000A LINE FEED (LF) or U+000D CARRIAGE RETURN (CR) characters into the value.
"""

So, we could simply have .value returning something else than the edited value if we believe that the edited value meant to be an URL.

However, that's not a trivial bug to solve because our current code isn't ready to show something that isn't the current input's value.

Comment 9

[Andrea Marchesini [:baku]]

Attachment: patch

Mounir, this patch is just a test. but it works..
Can you tell me why this is not enough?

Comment 10

[Mozilla RelEng Bot]

Try run for d7e004008ec8 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=d7e004008ec8
Results (out of 250 total builds):
    success: 197
    warnings: 53
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-d7e004008ec8

Comment 11

[Masatoshi Kimura [:emk]]

(In reply to Andrea Marchesini (:baku) from comment #9)
What happens if the value starts with "ftp:"?

Comment 12

[Mozilla RelEng Bot]

Try run for d7e004008ec8 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=d7e004008ec8
Results (out of 251 total builds):
    success: 197
    warnings: 53
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-d7e004008ec8

Comment 13

[Mounir Lamouri (:mounir)]

Comment on attachment 678148 [details] [diff] [review]
patch

Review of attachment 678148 [details] [diff] [review]:
-----------------------------------------------------------------

The concept of value mode comes from the specifications. We shouldn't change that to add our own hacks. The idea is to make this code readable for people who read the specs.

Also, I'm not a big fan of this hack in general. It might work but I wonder how much important it is to fix this bug. Adding a dirty hack might not worth it if we consider this bug as minor. Note that bug 805049 would introduce a real solution to fix this one.

Comment 14

[Andrea Marchesini [:baku]]

Attachment: patch 2

This second approach is less invasive. If the value is already a valid URL, this is not changed, otherwise, we try to make it a http URL.
This patch doesn't fix the mochitests. It's just a low priority piece of code. If we like how this fixes the problem, then I send a proper patch.

Comment 15

[Mounir Lamouri (:mounir)]

Andrea, is this patch passing try?

Comment 16

[Andrea Marchesini [:baku]]

The only bit problem I see is with the reftests because:

<input type="url" value="foo"> is not longer 'invalid'. because input.value is http://foo.
Then the background color is green, and the reftest fails.

In order to make it invalid we should have:
<input type="url" value="foo bar">
but again, reftest compares the images and foo != foo bar, so it fails again.

I prefer to have a feedback about this patch before fixing this reftests.

Comment 17

[Mounir Lamouri (:mounir)]

Comment on attachment 678760 [details] [diff] [review]
patch 2

Review of attachment 678760 [details] [diff] [review]:
-----------------------------------------------------------------

I am against introducing a hack to solve that bug. The real solution to solve this is to make nsHTMLInputElement able to have a value that isn't the one being shown by the frame (nsTextControlFrame). That would handle cases where the value is set by script. IOW, input.value = input.value; should be a no-op. With your patch, it is not but with a content/frame value differentiation, it would be.

Comment 18

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.