Posted by Dave Reid on May 31, 2011 at 8:30pm
Issue Summary
Split from #675348: META: Support HTML5 form input elements to add a new 'urlfield' Form API element.
Questions:
1. Do we add validation?
2. Should local/internal path textfields be converted to an urlfield (e.g. URL alias field on nodes)?
Comments
#1
#2
We probably should add validation, same validation thats done in the browser.
We shouldn't use this for local/internal path textfields mainly because the url field is supposed to be an absolute URL so if we use it for local/internal path the field will be :invalid.
#3
Yeah I double checked and the url element can only be used for absolute URLs, so that's a pretty easy decision as to how we validate it then: valid_url($url, TRUE).
#4
#5
The last submitted patch, 1174628-html5-url-element.patch, failed testing.
#6
#7
Incorrectly named validation function.
#8
The last submitted patch, 1174630-html5-url-element.patch, failed testing.
#9
#10
The last submitted patch, 1174630-html5-url-element.patch, failed testing.
#11
#12
Dropbox did not sync in time...
#13
Does the maxlength of 128 even need to be defined? If so, it should probably be longer than that, as some URLs are quite long.
#14
I'd be fine going with a default of 255 since that makes sense, but this was the default of the existing element textfield which was used for URLs.
#15
#16
@Dave, do we want to add the autocomplete to the url elements? I think it might be better to just leave the autocomplete stuff to the regular text fields.
We can always add them later.
#17
I don't see why not since this is essentially a text field anyway, it should probably support autocompletion.
#18
#19
Autocomplete on a URL field can cause a problem with iOs devices ... if autocomplete is on, it'll treat it as a semantic text field and offer suggestions from the dictionary - basically, they equate "autocomplete" with "autocorrect". The problem is that not only will it try to autocorrect domains to similar dictionary words, but if you cancel them out you'll get domains (or domain fragments) in your autocorrect dictionary.
I realise this is technically a problem for iOs rather than Drupal/HTML5, but I'd prefer to see autocomplete disabled on URL fields by default.
#20
subscribing for #1239910: META: tracking other issues about validation
#21
#22
Cleaning up tags.
#23
@kingandy
Can you please provide documentation on the iOS autocomplete issue on URLs? My suspicion is that this problem is when the @autocomplete attribute is set to "on", or rather not set to "off". This likely has nothing to do with our autocomplete implementation in autocomplete.js.
That is, we can set @autocomplete="off" on the field, and still allow lookup values from the server w/ autocomplete.js.
#24
I think you're right, yes. Since this was an HTML 5 thread, and I'd just come from doing some @autocomplete integration myself, I assumed that's what was under discussion and completely forgot the word was also used for Drupal's javascript/callback-based mechanism...
So, yes, switching off the user-agent based autocomplete and injecting our own suggestions via the established javascript should be completely fine :)
#25
Rerolled for core/ and the death of
profile.module.#26
This issue has actually been block by #1174634: Add new HTML5 FAPI element: telephone
We probably should tackle that one pretty soon, and wrap up all these form elements.
#27
Here's another rerolled this time with a couple of extra test.
This is probably going to need to be rerolled yet one more time after #1174634: Add new HTML5 FAPI element: telephone gets in but lets see if this passes with the extra test.
#28
Make sure to check this CSS too :)
#29
Yea, lol I wanted to make sure I got the test done right 1st, because I'm pretty sure i screw something up. Adding the css now.
#30
Side note that I think we may also need to add a selector for this in core/misc/vertical-tabs.css for
.vertical-tabs .form-type-url input.#31
The last submitted patch, 1174630-27.patch, failed testing.
#32
#33
So I tested this and the browser actually lets you submit with extra space on the url field so the trim makes sense here.
So this looks good to me. Marking it RTBC.
#34
We're missing tests to confirm the trimming behavior like are present on the e-mail patch.
+++ b/core/modules/simpletest/tests/form_test.moduleundefined
@@ -1205,6 +1205,14 @@ function _form_test_disabled_elements($form, &$form_state) {
);
}
+ // URL.
+ $form['disabled_container']['disabled_container_url']= array(
+ '#type' => 'url',
+ '#title' => 'url',
+ '#default_value' => 'http://example.com',
+ '#test_hijack_value' => 'http://example.com/foo',
+ );
+
// Text format.
$form['text_format'] = array(
'#type' => 'text_format',
What is this addition? I couldn't find anything similar in the 'tel' FAPI patch that went in?
#35
Yea, that extra content doesn't belong there.
This should be in the loop instead. It also looks like we missed a test in the tel element.
#36
Actually it's still missing the trim test.
#37
I was wrong - that hunk of code is needed since we can't use element value or hijack values of non-URL values otherwise the form submit fails validation. Sorry ericduran. :(
#38
Adding to current sprint. So in summary, we still need tests to confirm validation and trimming of this field.
#39
Yep, exactly. So ingnore #35 or were there any other changes?
#40
We're still missing a test to confirm that the form submits a trimmed URL field value.
#41
Of course. Thus "+Needs tests". I was talking about the patch in #35.
#42
Ok. Here are some tests.