[ticket/11010] HTML5 input types for form fields #1000

senky · 2012-09-30T18:14:04Z

The proposal is to use the new input types for form fields(new HTML5 feature). The idea is to use types like email and url to the browser automatically validate the fields.

RFC & discussion: http://area51.phpbb.com/phpBB/viewtopic.php?f=108&t=43159

http://tracker.phpbb.com/browse/PHPBB3-11010

PHPBB3-11010

brunoais · 2012-09-30T22:53:36Z

Interesting... It's the 1000th PR! PARTY!

p · 2012-10-11T02:30:49Z

phpBB/language/en/posting.php

@@ -161,7 +161,7 @@
 	'PLACE_INLINE'				=> 'Place inline',
 	'POLL_DELETE'				=> 'Delete poll',
 	'POLL_FOR'					=> 'Run poll for',
-	'POLL_FOR_EXPLAIN'			=> 'Enter 0 or leave blank for a never ending poll.',
+	'POLL_FOR_EXPLAIN'			=> 'Enter 0 for a never ending poll.',


What is the initial value? If initial value is empty and user is required to enter 0, I don't think this is acceptable.

Initial value is 0. Also, if you post a blank string, it gets replaced by 0. So that is why.

p · 2012-10-11T02:35:01Z

Two questions:

What happens if an older browser encounters this markup? And by "older" I mean 1) firefox 3 and 2) links/lynx.
I assume styles team approves of this PR?

p · 2012-10-11T02:36:43Z

Just noticed that subsilver2 received some of these changes, 1) do we really want that and 2) I seem to recall subsilver staying html4, is this going to be an issue?

senky · 2012-10-11T18:36:23Z

Inputs of unknown types falls back to type="text", so no harm is made. It was discussed in RFC.

I am not sure about subsilver2 then. Vinny did the original changes and included subsilver2, but if you want, I can delete commits that change subsilver2.

vinny · 2012-10-16T17:48:37Z

What happens if an older browser encounters this markup? And by "older" I mean 1) firefox 3 and 2) links/lynx.

@p the field will be analyzed as normal, ie as a simple text input.

Just noticed that subsilver2 received some of these changes, 1) do we really want that and 2) I seem to recall subsilver staying html4, is this going to be an issue?

The current subsilver2 is with HTML5 (<!DOCTYPE html>), however the new HTML5 input types usually work, only need the browser support.

p · 2012-10-17T02:20:09Z

OK then, sounds like you have all the bases covered. Please take out the .orig file.

p · 2012-10-22T16:10:49Z

In 3b2c53d: second line of commit message is not space: and removing it from prosilver wher it should not be.

p · 2012-10-22T16:28:07Z

There were 2 failures:

phpbb_functions_acp_build_cfg_template_test::test_build_cfg_template_dimension with data set #0 (array('dimension', 20, 255), 'key_name', array(10, 20), 'config_key_name', array(), ' x ')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-' x '
+' x '

/home/phpdev/test-phpbb/tests/functions_acp/build_cfg_template_test.php:89
/home/pie/apps/git-phpunit-bundle/phpunit.php:44

phpbb_functions_acp_build_cfg_template_test::test_build_cfg_template_dimension with data set Develop olympus #1 (array('dimension', 0, 255), 'key_name', array(10, 20), 'config_key_name', array(), ' x ')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-' x '
+' x '

/home/phpdev/test-phpbb/tests/functions_acp/build_cfg_template_test.php:89
/home/pie/apps/git-phpunit-bundle/phpunit.php:44

p · 2012-10-22T16:28:36Z

Someone needs to fix the test suite, the rest of it looks a-ok to me.

senky · 2012-10-22T18:45:26Z

I would like to reword 3b2c53d, but when I do git rebase, with its parent, it wants me to rebase more than 200 commits, more of them with problems which interupts rebasing process. Could you lead me on how to reword that old commit without that much pain using git rebase? I could not find anything on the internet...

p · 2012-10-22T19:54:23Z

That is probably anywhere between difficult and impossible, you can leave it alone.

p · 2012-10-22T19:55:11Z

The simplest thing probably would be to start on current develop and cherry-pick each commit in this PR. If you get major conflicts, again, you can leave it be.

p · 2012-10-22T19:57:19Z

Actually it doesn't look so bad.

git checkout ticket/11010
git rebase -i upstream/develop

Delete any commits not listed in this PR.

Fix conflicts, there should only be 1 or 2.

If you do this please squash the commit deleting the orig file into the commit that added it.

After you do this, rebase again and reword the offending commit.

senky · 2012-10-23T10:18:57Z

In fact it is a problem. I did everything according to your instructions. There were 2 conflicts. I resloved them. I could not squash commit, because I cannot find commit adding .orig file. Whatsmore, After I finished rebasing, it resulted in error, that ref cannot be updatet because it should be on another commit that it is:

error: Ref refs/heads/ticket/11010 is at 9bcd9bbd5bf97d824a53735de3ceca82540dfea
5 but expected 9c7afaaf8576482b034def672bae4ca783b3a960
fatal: Cannot lock the ref 'refs/heads/ticket/11010'.

I fixed it by modifying commit ID in .git/refs/heads/ticket/11010, but after I tried to rebase it again to reword the commit, it gave me hute lost of commits again.

Well, I do not know it it is possible to reword it. I have another idea: I can just simply copy all files and create new PR, include only one commit adding everything in this PR.

p · 2012-10-23T17:10:17Z

Most likely you did not start with a clean tree/staging area/rebase state etc. and that is why you received that error.

Make sure you completely cancel out any rebases in progress and your repository is in good shape.

Then rebase this branch on develop without any changes to commits. Only fix the conflicts. Checkpoint the result.

Then rebase and reword the commit.

Then rebase and squash orig delete. To find out which commit added it:

git blame HEAD^ path.orig

after you do all the other steps.

senky · 2012-10-28T12:35:34Z

Ok I give up. I tried to do it with your instructions, but after I fixed conflicts and ran rebase again, the same conflicts appeared (any yes, I used git add .). I have given you collab access to my fork, so if you have time, I would be glad to see it fixed and merged.

p · 2012-10-29T11:54:50Z

phpBB/includes/functions_acp.php

-			$size = (int) $tpl_type[1];
-			$maxlength = (int) $tpl_type[2];
+			$min = (int) $tpl_type[1];
+			$max = $maxlength = (int) $tpl_type[2];


This was an API change. The first parameter was size and is now min.

I don't see dimension being used anywhere in phpbb tree, therefore you may be able to do this but you need to get approval from more developers.

It's okay with me.

This is still not right.

min/max refer to the magnitude of the value, maxlength refers to the number of digits in the value. For a max=1000, maxlength=4.

maxlength could perhaps be a ceil(log10(max)).

Behavior like this should not change, it will break any mod that uses this in the ACP and it will probably not be the easiest to recognize why this changed.

Doesn't number do what you changed dimension to?

p · 2012-10-29T11:55:30Z

I did the first rebase, you can now reword that commit.

Tests fail due to an API change - see my previous comment.

p · 2012-10-29T11:56:21Z

In 5a936918d1c6a8b912553486cda7d2ebf49249eb: second line of commit message is not space: and removing it from prosilver wher it should not be.

senky · 2012-10-29T19:51:08Z

Ok I tried, even though some problems occured. I hope it is fine now. And about your comment to dimension: you have already fixed the problem with a14aaee or am I to fix it?

p · 2012-11-06T17:18:11Z

The last merge you did you should not have done.

git checkout ticket/11010
git reset --hard a14aaee50fc4e5beb5df1da4f26873dbbe510488

As I said, you changed the API which requires discussion and approval. You can either revert to the old API which will require changes in the diff, or post an RFC for the API change you have done.

senky · 2012-11-06T19:22:37Z

Here is the RFC:
http://area51.phpbb.com/phpBB/viewtopic.php?f=108&t=43569

p · 2012-12-02T01:38:34Z

This massively conflicted with the colon merge.

p · 2012-12-06T08:01:35Z

@senky please merge or rebase on develop to deal with the colon conflicts.

senky · 2012-12-06T11:14:39Z

Ok I will try. If it will be too problematic, I will maybe rewrite all changes to newest source and push it here.

senky · 2013-01-07T09:48:04Z

I have successfully rebased on develop and resolved all conflicts with L_COLON. I hope it is ready to merge now.

PHPBB3-11010

Add new HTML5 input types to installer PHPBB3-11010

Add all new HTML5 input types to functions_acp.php, handle them as text for now PHPBB3-11010

Change input type to search for search forms Replace search placeholder JS with placeholder attribute Add style for search placeholder PHPBB3-11010

PHPBB3-11010

Add type="email" to fields that should be email fields Change back to type="text" fields that might have wildcards or not necessary email PHPBB3-11010

PHPBB3-11010

senky · 2013-05-20T19:19:28Z

@EXreaction tests are failing, but after inspection, you can see, that the problem is in downloading dependencies, so travis problem. Tests should pass.

EXreaction · 2013-05-20T20:34:06Z

phpBB/includes/acp/acp_board.php

@@ -399,8 +399,7 @@ function main($id, $mode)
 						'legend1'				=> 'ACP_SECURITY_SETTINGS',
 						'allow_autologin'		=> array('lang' => 'ALLOW_AUTOLOGIN',		'validate' => 'bool',	'type' => 'radio:yes_no', 'explain' => true),
 						'allow_password_reset'	=> array('lang' => 'ALLOW_PASSWORD_RESET',	'validate' => 'bool',	'type' => 'radio:yes_no', 'explain' => true),
-						'max_autologin_time'	=> array('lang' => 'AUTOLOGIN_LENGTH',		'validate' => 'int:0',	'type' => 'text:5:5', 'explain' => true, 'append' => ' ' . $user->lang['DAYS']),
-						'ip_check'				=> array('lang' => 'IP_VALID',				'validate' => 'int',	'type' => 'custom', 'method' => 'select_ip_check', 'explain' => true),


What happened to this setting?

EXreaction · 2013-05-20T20:53:59Z

Besides the config setting that went missing, I think everything looks acceptable. If someone else would please review all the changes as well and respond back if they think everything looks good that would be very helpful since there were so many changes here and it's easy to miss something.

gooof · 2013-05-20T23:39:52Z

phpBB/install/index.php

+			case 'range':
+			case 'search':
+			case 'tel':
+			case 'time':


PHPBB3-11010

senky · 2013-05-21T09:59:00Z

Thanks for your comments. Problems have been resolved. I hope now it is ok.

cyberalien · 2013-05-21T13:26:44Z

Looks good. Can't find any errors

EXreaction · 2013-05-21T14:52:29Z

Merged, thanks for all the hard work!

senky · 2013-05-21T15:40:59Z

Yeah! Finally after 8 months :). Thanks to all collaborators!

senky mentioned this pull request Oct 2, 2012

[ticket/11010] adding type="url" to 'site_home_url' #1003

Closed

p reviewed Oct 11, 2012
View reviewed changes

p reviewed Oct 29, 2012
View reviewed changes

senky and others added 17 commits May 20, 2013 18:40

[ticket/11010] adding type="url" to 'site_home_url'

f75aa97

PHPBB3-11010

[ticket/11010] Progress toward fixing the test suite.

bee2faa

PHPBB3-11010

[ticket/11010] Mock user in order for the test to run standalone.

84fb8fc

PHPBB3-11010

[ticket/11010] Try to keep min < max.

4bb7a69

PHPBB3-11010

[ticket/11010] applying some of EXreaction notes

cf9d407

PHPBB3-11010

[ticket/11010] replacing maxlenght with max

6c0f351

PHPBB3-11010

[ticket/11010] fixing tests

77ae733

PHPBB3-11010

[ticket/11010] displaying min and max only if they are set

05639b8

PHPBB3-11010

[ticket/11010] validation to match type

3547b97

PHPBB3-11010

[ticket/11010] Add HTML5 input types to installer

854b88d

Add new HTML5 input types to installer PHPBB3-11010

[ticket/11010] Add all HTML5 input types to ACP

06b7e42

Add all new HTML5 input types to functions_acp.php, handle them as text for now PHPBB3-11010

[ticket/11010] Change input type to search for search forms

178f23d

Change input type to search for search forms Replace search placeholder JS with placeholder attribute Add style for search placeholder PHPBB3-11010

[ticket/11010] dealing with dimensions problem, removing extra code

4c03c22

PHPBB3-11010

[ticket/11010] updating tests

ae2700e

PHPBB3-11010

[ticket/11010] Fix email inputs

504038d

Add type="email" to fields that should be email fields Change back to type="text" fields that might have wildcards or not necessary email PHPBB3-11010

[ticket/11010] Fix input type for urls

cfa23c1

PHPBB3-11010

[ticket/11010] Fixes for number input in acp

3e64c1b

PHPBB3-11010

EXreaction reviewed May 20, 2013
View reviewed changes

gooof reviewed May 20, 2013
View reviewed changes

phpBB/install/index.php

case 'range':

case 'search':

case 'tel':

case 'time':

Copy link

gooof May 20, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done twice

senky added 2 commits May 21, 2013 11:55

[ticket/11010] resolving latest comments

930f4c7

PHPBB3-11010

[ticket/11010] properly tabifying 'max_autologin_time'

721bc03

PHPBB3-11010

EXreaction merged commit 721bc03 into phpbb:develop May 21, 2013

[ticket/11010] HTML5 input types for form fields #1000

[ticket/11010] HTML5 input types for form fields #1000

Conversation

senky commented Sep 30, 2012

brunoais commented Sep 30, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

p commented Oct 11, 2012

p commented Oct 11, 2012

senky commented Oct 11, 2012

vinny commented Oct 16, 2012

p commented Oct 17, 2012

p commented Oct 22, 2012

p commented Oct 22, 2012

p commented Oct 22, 2012

senky commented Oct 22, 2012

p commented Oct 22, 2012

p commented Oct 22, 2012

p commented Oct 22, 2012

senky commented Oct 23, 2012

p commented Oct 23, 2012

senky commented Oct 28, 2012

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

p commented Oct 29, 2012

p commented Oct 29, 2012

senky commented Oct 29, 2012

p commented Nov 6, 2012

senky commented Nov 6, 2012

p commented Dec 2, 2012

p commented Dec 6, 2012

senky commented Dec 6, 2012

senky commented Jan 7, 2013

senky commented May 20, 2013

Choose a reason for hiding this comment

EXreaction commented May 20, 2013

Choose a reason for hiding this comment

senky commented May 21, 2013

cyberalien commented May 21, 2013

EXreaction commented May 21, 2013

senky commented May 21, 2013