Problem/Motivation

This method uses the html_time format to create the date object from user input.

Firstly, if the time format does NOT include seconds, but the time input does, then the object instantiation fails. In this case the seconds should be trimmed from the input before object instantiation.

Secondly, this method makes an assumption that a five character time value is always invalid and then concatenates `:00`

If the html_time format has been altered to omit the seconds, e.g. `H:m`, and the element has correctly set `$element["#date_increment"] = 60;` so that the input length is 5 to match the time_format, then this assumption is false. Again, the call to `DrupalDateTime::createFromFormat` throws. This makes the `object` in the value array `NULL` instead of a valid date object.

In both cases, the datetime element validation fails with the message "The %field date is invalid. Please enter a date in the correct format.", when in fact the datetime input is valid.

Steps to reproduce

  1. Change the html_time format to `H:m`
  2. Try to submit a form having a datetime field with $element["#date_increment"] = 1
  1. Change the html_time format to `H:m`
  2. Try to submit a form having a datetime field with $element["#date_increment"] = 60

Proposed resolution

Replace the current assuming code with a section that takes into account the time_format

Remaining tasks

1. Add test coverage

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3591711

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

aklump created an issue. See original summary.

aklump’s picture

Issue summary: View changes
aklump’s picture

I'm not clear on how to add the test coverage for this, and I'm not able to devote the time to figuring that out at this moment. I'm hoping someone else can take that baton and finish the race. Thanks in advance!

cilefen’s picture

Issue tags: +Needs merge request
aklump’s picture

@cilefen Should I make the MR BEFORE test(s) get added? I was thinking to wait...?