Problem/Motivation

Since support was added to core for relative default dates, core seems to support relative dates + times as well. There is just a problem with the timezone handling.

Timezone bug picture

Steps to reproduce

Set your site to a another time zone as GMT (for example GMT+8) and create a date field on a content type with a relative default date and time (+1 Saturday 13:00). When adding content, the default date might be correct, but the time has "updated" to the timezone (21:00).

Proposed resolution

Looks like there might be some timezone overcompensation. Patch attached

Remaining tasks

  • Code review
  • Expand test coverage for daterange default value

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

Issue fork drupal-3169876

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

splash112 created an issue. See original summary.

splash112’s picture

StatusFileSize
new2.69 KB
splash112’s picture

Version: 8.9.x-dev » 9.0.x-dev
Category: Feature request » Bug report
Issue summary: View changes
StatusFileSize
new27.7 KB

Checked again at a fresh install of Drupal 9 and issue is still there. Proposed solution to remove timezone compensation from the datetime and datetime_range default options might not be optimal, but works.

Testbot seems to like to counterintuitive way of handling the time zone correction though.

Version: 9.0.x-dev » 9.1.x-dev

Drupal 9.0.10 was released on December 3, 2020 and is the final full bugfix release for the Drupal 9.0.x series. Drupal 9.0.x will not receive any further development aside from security fixes. Sites should update to Drupal 9.1.0 to continue receiving regular bugfixes.

Drupal-9-only bug reports should be targeted for the 9.1.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.2.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.1.x-dev » 9.3.x-dev

Drupal 9.1.10 (June 4, 2021) and Drupal 9.2.10 (November 24, 2021) were the last bugfix releases of those minor version series. Drupal 9 bug reports should be targeted for the 9.3.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

jprj made their first commit to this issue’s fork.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

ericgsmith made their first commit to this issue’s fork.

ericgsmith’s picture

Converted patch to MR - curious to see what the tests will show.

I'm not sure the possible side effects of the change, but for me it doesn't make sense to use the storage timezone for relative dates in the UI - its confusing and limits some good meaning defaults we can have.

Found this issue after what seemed like a very simple request from a client "we want the default value to be the next day at 2pm" - can't do it as even doing hours relative to UTC is impacted by timezone changes.

A relative value of "tomorrow 2pm" works with this patch.

ericgsmith’s picture

Status: Active » Needs work

Not tests fail with this change - interesting!

Looks like DateTimeFieldTest::testDefaultValue only tests for date only fields which have not changed here - it will need some test coverage for datetime.

ericgsmith’s picture

Issue summary: View changes

I have expanded DateTimeFieldTest::testDefaultValue to also test using a datetime field.

I note that there is currently no coverage in the daterange test for timezones or using the datetime type - I am assuming this will need to also get coverage to change - but setting to needs review to get feedback on this change in general.

ericgsmith’s picture

Status: Needs work » Needs review
rosk0’s picture

Status: Needs review » Needs work
ericgsmith’s picture

Adding related issue #2778083: Default value for Date-Only fields is broken when UTC date is different than user current date - this is where the bug was fixed for date only fields - it is not clear in that issue why datetime was excluded from this fix

ericgsmith’s picture

Status: Needs work » Needs review

Addressed MR feedback and expanded range test to match date time field.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

believe feedback for this one has been addressed.

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new2.52 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

ptmkenny made their first commit to this issue’s fork.

ptmkenny’s picture

I added the missing return type as suggested in #20, but I can't determine why the unit tests are now failing.

ericgsmith’s picture

Issue summary: View changes
ptmkenny’s picture

Status: Needs work » Reviewed & tested by the community

It looks like the test failure was temporary, as the tests are now passing again. I think it's also highly unlikely that adding a return type of array to a function that always returned an array will break anything, so I'm setting this back to RTBC.

alexpott’s picture

Status: Reviewed & tested by the community » Needs review

This bug feels tricky... it you set the default relative date to +1 Saturday 13:00 whose timezone does the 13:00 apply to? The user who is creating the content - the site? I agree that there is a bug here because UTC is not the correct choice for default values. But I'm not sure that user's timezone is either. I think I would expect the site's timezone to be respected. So no matter by whom the content is created the default time is the same. Tricky. Note that if user timezones are not configurable I think this is exactly what this MR delivers.

rkoller’s picture

From a users perspective this is super confusing. i've tested after reading #25. My setup:

admin/config/regional/settings has New York as the default time zone
user/1/edit has Berlin as the time zone in the local settings
user/2/Edit has New York as the default time in the local settings
+1 Saturday 13:00 is the relative default value set on the date field

Without the MR on the node edit form (creator user 1) :
11/16/2024 2pm (user 1 with berlin)
11/16/2024 8am (user 2 with new york)

with the MR on the node edit form (creator user 1)
11/16/2024 1pm (user 1 with berlin)
11/16/2024 7am (user 2 with new york)

I receive the same set of dates and times if i create the node with user 2 same as if i create the field and set the relative date with user 2. that leaves me as the user puzzled and completely in lack of situational awareness about what the actual point of reference is when i am setting the relative default value on the field? my main assumption would have been that the point of reference date and time would be new york, the sites timezone. but with +1 saturday 13:00 against new york i wouldnt expected to get 11/16/2024 7am (with MR) 11/16/2024 8am (without MR); for user 2 i would have expected those time and for user 1 i would have expected 7pm (with MR) and 8pm (without MR) but not the other way around?

I think one of the most important things here is providing the actual point of reference for the calculations on the description of the "relative default value" field on the field settings to provide some situational awareness to the user. at the moment the description only provides

Describe a time by reference to the current day, like '+90 days' (90 days from the day the field is created) or '+1 Saturday' (the next Saturday). See strtotime for more details.

there is no clue about the point of reference the calculations are based on? personally, as a user, i am completely lost here and feel highly confused.

rkoller’s picture

might be also a more than suitable issue to discuss on fridays ux meetings imho. shall i add it to the shortlist?

alexpott’s picture

@rkoller thanks for the fantastic testing and yes I think discussing as a group is the best way forward.

rkoller’s picture

i've added the issue to the shortlist yesterday and maybe we find time already today discussing it. But I did some more testing and the setup i've did yesterday was missing an important aspect - it matters who creates the issue. The matter is still puzzling, well time zones in general, but at least things became a bit more clear now. The revised setup:

admin/config/regional/settings has New York as the default time zone
user/1/edit (rkoller) has Berlin as the time zone in the local settings
user/2/Edit (admin) has New York as the default time in the local settings
+2 Saturday 13:00 is the relative default value set on the date field

No MR applied

Node creator: rkoller - Berlin

Node edit form:
11/23/2024 02:00:00pm

Node:
Sat, 23 Nov 2024 - 14:00 (rkoller - Berlin)
Sat, 23 Nov 2024 - 08:00 (admin - New York)

Node creator: admin - New York

Node edit form:
11/23/2024 08:00:00am

Node:
Sat, 23 Nov 2024 - 14:00 (rkoller - Berlin)
Sat, 23 Nov 2024 - 08:00 (admin - New York)

MR applied

Node creator: rkoller - Berlin

Node edit form:
11/23/2024 01:00:00pm

Node:
Sat, 23 Nov 2024 - 13:00 (rkoller - Berlin)
Sat, 23 Nov 2024 - 07:00 (admin - New York)

Node creator: admin - New York

Node edit form:
11/23/2024 01:00:00pm

Node:
Sat, 23 Nov 2024 - 19:00 (rkoller - Berlin)
Sat, 23 Nov 2024 - 13:00 (admin - New York)

rkoller’s picture

*deleted the duplicate posting. ran into a 5xx error when posting and that lead into the duplicate post, apologies.

alexpott’s picture

I think the MR behaviour is way better than HEAD because it is consistent. On the node edit form the time being saved is in the users timezone which makes sense. And then when a user views it they see the correct time. I think the major UX issue is that we're not showing the timezone on the node edit form as that would be very very helpful for people because maybe the node creator rkoller is very aware the site's timezone is new york and would expect node edit forms to be in the site timezone and not theirs.

sagarmohite0031’s picture

StatusFileSize
new61.38 KB
new60.58 KB

Hello,
MR applied successfully attaching before and after screenshots.
Please check attachments

rkoller’s picture

StatusFileSize
new14.76 KB

Usability review

We discussed this issue at #3486279: Drupal Usability Meeting 2024-11-15. The direct link to the recording of the meeting is https://www.youtube.com/watch?v=Yy18FL7GuvE. For the record, the attendees at the usability meeting were @AaronMcHale, @benjifisher, @rkoller, and @simohell.

First we went through the tests outlined in #29, comparing the behavior of the relative default value field with and without the MR applied. We asked ourselves if the root cause for the odd behavior is the default value or how the field is getting interpreted, so we went ahead and did a few more experiments. First we tried adding a timezone to the relative default value field (which is not directly apparent) and noticed a few problems along the way:

Entering +2 Saturday 13:00 America/New York lead to the following error: The relative date value entered is invalid. Although the error message explains what went wrong, but at the moment it does not provide any solution how to resolve the invalid date value error - we have tried different variants of lower and upper case lettering for “New York” but no luck. In our explorations following up after the meeting we’ve realized that the timezone does not allow any spaces, you have to use an underscore, like +2 Saturday 13:00 America/New York, to be recognized as a valid date value.

We’ve noticed another detail about the capitalization of timezones. +2 Saturday 13:00 GMT+5, +2 Saturday 13:00 gmt+5, +2 Saturday 13:00 Gmt+5, +2 Saturday 13:00 UTC+5, +2 Saturday 13:00 utc+5, or +2 Saturday 13:00 Utc+5 are all recognized as valid when then field settings are saved. Problem is if you go to the corresponding node edit forms that are using the field with those timezones on the relative default values, the set default times are only shown for the upper case variants UTC and GMT, all other cases like gmt+5, Gmt+5, utc+5,and Utc+5, lead to empty fields only showing a placeholder value instead:

date field set with empty date and time field that only show greyish hard to read placeholder text

We then followed the strtotime-link in the description for the Relative default value field which lead to the documentation page https://www.php.net/manual/en/function.strtotime.php for that PHP-function. The page contains an explicit warning that the timestamp that this function returns does not contain any information about timezones.

Now that time zones are implicitly and explicitly used in the Relative default value field, we wondered how date and time is actually stored in the database. Turns out the default relative date and time value is stored as a string (for example +1 Saturday 23:00 or +2 Saturday 13:00 UTC+1), and on the particular node it is also stored as a string (for example 2024-11-16T23:00:00). So for the field settings the timezone is being stored if added by the user, while the string stored on the node is not containing any information about the timezone.

Due to subsequent discussions to the meeting in the #ux channel on the Drupal Slack, I’ve further expanded the test setup from #29 to better understand the actual behavior: https://gist.github.com/rpkoller/9b4c93c28d1d97ccec404194e277f404. The first markdown file is the scenario from #29, for the second markdown file an explicit timezone (America/Los_Angeles) dissimilar to the site's and the user's was used, and the last three scenarios explicitly applied UTC and the timezones of the two users to the default relative date.
It turns out that if no timezone is explicitly set for the default relative date, Drupal is using an implicit timezone. Without the MR applied, UTC is used, while with the MR applied, the timezone of the current user is used. So the relative default value is using a timezone all the time, either implicit and not shown to the user or explicit if set and therefore changed by the user.

After getting an overview of the entire problem space, we came to the following initial conclusions:

  • The currently implicit timezone for the relative default value should be made explicit and actually shown (*we haven't discussed the details about how the field should behave and be presented yet, but one thought that came to my mind during finalizing this comment- if the user isn't entering and altering the default timezone the default timezone should be added to the relative default value on safe). The site builder needs to be aware of the point of reference, no matter if the MR will use in the end the site’s timezone or the user’s timezone - each of the two options has a reasonable use case. The user simply has to be aware THAT a timezone is used and which it actually is.
  • In the context of the timezone the problems listed above in regard of case and syntax should be tackled, to avoid those form errors as well as empty date fields on node edit forms that are missing the set relative default date and time.
  • The description for the relative default value field should also be extended, perhaps providing an example based on the timezone for the relative default value and providing one or two example about the actual values shown on the node edit form to illustrate the implications.

In addition to that @benjifisher tried to get some advice from @mandlcu, as the maintainer of the smart date module, at Nedcamp and will add the outcome in a comment on this issue.

If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.

rkoller’s picture

Due to the additional research and discussion, and the complexity of the topic the write up of the comment took a little bit longer. But I agree with the point @alexpott meanwhile made in #31, it is not only necessary to add the timezone to the default value on the field settings page, but also sort of required to add the point of reference aka the timezone to the node edit form as well. Otherwise the entry the user makes on the node edit form is just based on an assumption. That is also sort of in line with an article i’ve stumbled across a few days ago: https://simonwillison.net/2024/Nov/27/storing-times-for-human-events/. In the recommendation section the authors suggests to store the user’s intent time and the location/timezone. I suppose that suggestion is out of the scope for this issue but I consider it a more than reasonable one, and would suggest to open up a followup for it?

smustgrave’s picture

@alexpott based on #31? Is this a net gain enough to move forward?

smustgrave’s picture

Issue tags: +Needs followup

Rebased as it was 590 commits back. Still green.

@rkoller do you want to open the follow up you mentioned?

Seems like there could be some net gain here even if not 100% perfecrt.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Since this is a net gain going to mark, will ping rkoller about the follow up if needed

ptmkenny’s picture

Issue tags: +Needs change record

I think this definitely needs a change record because this MR changes timezone behavior.

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

ptmkenny’s picture

Status: Needs work » Needs review

Fixed phpcs and setting back to "needs review". We still need a change record though. If someone who wrote the code writes a draft change record, I'm happy to clean it up.

ptmkenny’s picture

StatusFileSize
new62.86 KB

Patch for 11.2 for installing with composer. (only worked for alpha)

ptmkenny’s picture

smustgrave’s picture

Status: Needs review » Needs work

Changes are looking good, still just need the CR.

ptmkenny’s picture

Status: Needs work » Needs review
Issue tags: -Needs change record

I added a change record here: https://www.drupal.org/node/3527534

wim leers’s picture

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs change record updates

Sorry for the delay. For the CR can we provide some examples of what was happening before to be more clear. @rkoller do we still need a follow up?

rkoller’s picture

hm one follow up i've already created quite a while ago, the one referenced in the sidebar #3512375: [PP-1] Make the relative default value validation more forgiving towards deviating time zone notations. but the more important other one i am struggling to write up for a while now. problem is that the relative default date time using timezones implicitly ripples though the entire problem space. meaning it isnt only requiring a followup for the relative default date time field in the field settings but changes to other parts as well. i did some research and currently struggling to chop things up. probably the "easiest" might be to open a meta issue outlining the most pressing problems. then it can be decided how to proceed. will try to finish that hopefully on the weekend. but have to finish a few other things first.

ptmkenny’s picture

I create a new MR, 3169876-timezone-handling, because I was having trouble rebasing 3169876-better-handling-of to work on 11.x. The code is the same, but 3169876-timezone-handling applies correctly on 11.x.

ericgsmith changed the visibility of the branch 3169876-better-handling-of to hidden.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

ptmkenny’s picture

Status: Needs work » Needs review

I fixed the test, and I updated the change record with AI assistance (claude code). Setting back to "Needs review".

ptmkenny’s picture

smustgrave’s picture

This one still need a follow up?

ptmkenny’s picture

Hmm, it is tagged "Needs followup," and the last discussion of the follow-up is in #47. It would be great if @rkoller could chime in.