Date Only field shows incorrect default value when UTC date is different than user's date

Can we get a functional test (ie, a BrowserTestBase one) to demonstrate the problem? Kinda baffled that the TZ loop DatetimeFieldTest w/ dateonly fields doesn't catch this.

Confirmed the bug. The problem is the initial default value, when the node is new.

In looking at DateTimeFieldTest::testDefaultValue(), it looks like we are missing an assertion of the form value for the 'Current date' setting. In

      // Create a new node to check that datetime field default value is +90
      // days.
      $new_node = Node::create(['type' => 'date_content']);
      $expected_date = new DrupalDateTime('+90 days', drupal_get_user_timezone());
      $this->assertEqual($new_node->get($field_name)->offsetGet(0)->value, $expected_date->format(DateTimeItemInterface::DATE_STORAGE_FORMAT));

we don't test that through the widget, just the field.

That test class needs to be burninated.

Here's a start at a test. Looks like the patch works, except for for Pacific/Tongatapu (which is UTC+13). Haven't looked at that yet.

#9, yeah 'Pacific/Tongatapu' was specifically added the the date-only TZ test "loop" because it is permanent UTC+13 (extreme edge case, and Fiji is +12+13 depending on DST). I am just really baffled why we are seeing this now, and not when we fixed that mess in the first place.

Not sure if we have a valid solution. Right now (Tue Sep 11 00:52:56 UTC 2018), Pacific/Midway is failing locally.

#16 still fails for me locally (at Tue Sep 11 03:01:54 UTC 2018) at Pacific/Midway at

      $today_storage = $this->dateFormatter->format($request_time, 'html_date', NULL, DateTimeItemInterface::STORAGE_TIMEZONE);
      $this->assertEquals($today_storage, $node->field_dateonly->value);

Not sure, but this might be another issue that's fixed/improved with #3009377: Replace drupal_get_user_timezone with a date_default_timezone_get() and an event listener

#19 didn't fix this, I'm on 8.9.6 and this issue is still occurring.

Applying 16 fixed it.

Just mentioning similar issue at https://www.drupal.org/project/drupal/issues/3069854

Updated with info from #3069854: 'Current date' default for date only field can be off depending on time zone setting which I closed as a duplicate, and also adding credit from that issue.

I am unable to replicate the issue with timezone set to Los Angeles and working fine. Added Screenshot for the reference.

Shared the patch from #3069854 which was closed as duplicate.

9.2.x: PHP 7.4 & MySQL 8
Regional settings: Pacific/Auckland (+13:00)

I checked 2993165-31.patch It removes the code that changes the timezone, but the issue is still there.

I've tested 2993165-24.patch and it fixes the issue. That patch has been re-rolled in MR!1575.

No, patch #31 is not working correctly.

My timezone is +11hrs (Sydney)

Date-only date is loaded from UTC and converted to Sydney, so every time date is saved (before 11am local time) it's saved in UTC which is -1 day! See screenshot of saving a node twice changes the date.

Timezone of the date and of the widget should be always UTC to keep the date consistent.

It's a display issue of today's date (when server's date is on the next day from UTC).

I'll investigate further.

@jannakha Your experience is noted I think by @tatianag in comment 37 - so you may find that the open MR!1575 addresses your situation.

@tatianag and I are way ahead of you - that's a joke, because we're in UTC+13 ;)

Note that MR!1575 is more recent than patch #31.

MR link "Plain diff" under the issue description can be used in composer patches downloaded to your project to provide a patch that won't change unexpectedly (see #3204538: GitLab Merge Requests Unable to Generate Incremental Patch Files on this).

WOW cool! yes, I can see now!
perfect!
thank you, kiwi friends!

so patch #24 and MR!1575 is correct!

Tested and it works!
I hope it'll end up in the core soon.

MR link "Plain diff" under the issue description can be used in composer patches.

Public service announcement: You should never do this. Anyone can push more code to the MR, and on the next deployment your site could all of a sudden be broken, or worse, vulnerable to a backdoor, hack, etc. The only safe thing to do is download such a plain diff, audit it / verify it’s the code you want, commit the known-good version of the patch to your site’s Git repo and have composer apply the local patch.

Safe site building,
-Derek

Meanwhile, the MR is currently failing tests, so this can’t be RTBC. 😉

- Merged latest commits from 9.4.x-dev into MR
- Fixed deprecations
- TLC
- Added test-only patch

@spokje - thanks for the patch, nice work!

@dww - thanks for the clarification, agree and my comment intended to direct @jannakha to the availability of the patch was missing the additional guidance you added.

Attaching a copy of MR !1575 as of ca2bdea3 as Gitlab MR URLs are subject to unexpected changes (ref #3204538).

Quick re-roll for 9.5.x accounting for https://www.drupal.org/node/3168858

Back to NR

And back to NW, too quick :)

@ranjith do you want to update your patch to incorporate CR#3168858: drupalPostForm() in functional tests is deprecated instead?

Update dpatch for 9.4

Fixed Failed Commands #55

Now that New Zealand is in daylight savings time (UTC+13) it's possible to test this patch properly in the wild. I have it running on several production sites and we'll be rolling this out to others as needed.

I think we should reconsider the documentation of \Drupal\Component\Datetime\DateTimePlus::setDefaultDateTime() - on its own this method leads to this problem. It's called from a few places in core but this is the only place where the timezone on the date might not be UTC. The other runtime place is \Drupal\datetime\DateTimeComputed::getValue() - fwiw I'm not sure if implementation is correct. Dates and timezone stuff is hard to reason about. But this should be in a follow-up to improve the docs on setDefaultDateTime

Backported to 9.4.x because this is a non-disruptive bug fix.

Committed and pushed c9d63410bc to 10.1.x and e126af9cb0 to 10.0.x and 489730b19f to 9.5.x and b12a1586e3 to 9.4.x. Thanks!

diff --git a/core/modules/datetime/tests/src/Functional/DateTestBase.php b/core/modules/datetime/tests/src/Functional/DateTestBase.php
index ea76cd7898..57cc4cb641 100644
--- a/core/modules/datetime/tests/src/Functional/DateTestBase.php
+++ b/core/modules/datetime/tests/src/Functional/DateTestBase.php
@@ -23,11 +23,6 @@ abstract class DateTestBase extends BrowserTestBase {
    */
   protected static $modules = ['node', 'entity_test', 'datetime', 'field_ui'];
 
-  /**
-   * {@inheritdoc}
-   */
-  protected $defaultTheme = 'stark';
-
   /**
    * An array of display options.
    *
diff --git a/core/modules/datetime/tests/src/Functional/DateTimeWidgetTest.php b/core/modules/datetime/tests/src/Functional/DateTimeWidgetTest.php
index fe2399e96a..6f6e7449dc 100644
--- a/core/modules/datetime/tests/src/Functional/DateTimeWidgetTest.php
+++ b/core/modules/datetime/tests/src/Functional/DateTimeWidgetTest.php
@@ -13,6 +13,11 @@
  */
 class DateTimeWidgetTest extends DateTestBase {
 
+  /**
+   * {@inheritdoc}
+   */
+  protected $defaultTheme = 'stark';
+
   /**
    * The default display settings to use for the formatters.
    *

This shouldn't be added to the base class. Moved it to the new test.

diff --git a/core/modules/datetime/tests/src/Functional/DateTimeWidgetTest.php b/core/modules/datetime/tests/src/Functional/DateTimeWidgetTest.php
index 6f6e7449dc..529d06c96b 100644
--- a/core/modules/datetime/tests/src/Functional/DateTimeWidgetTest.php
+++ b/core/modules/datetime/tests/src/Functional/DateTimeWidgetTest.php
@@ -4,7 +4,6 @@
 
 use Drupal\field\Entity\FieldConfig;
 use Drupal\field\Entity\FieldStorageConfig;
-use Drupal\Node\Entity\Node;
 
 /**
  * Tests Datetime widgets functionality.
@@ -94,10 +93,7 @@ public function testDateonlyDefaultValue() {
       $this->submitForm($edit, 'Save');
       $this->assertSession()->pageTextContains('dateonly_content ' . $timezone . ' has been created');
 
-      preg_match('|node/(\d+)|', $this->getUrl(), $match);
-      $nid = $match[1];
-      $node = Node::load($nid);
-
+      $node = $this->drupalGetNodeByTitle($timezone);
       $today_storage = $this->dateFormatter->format($request_time, 'html_date', NULL, $timezone);
       $this->assertEquals($today_storage, $node->field_dateonly->value);
     }

Loaded the node in a better way that does not use a regular expression. Ran the test locally with this change and it works great.

Created #3313838: Improve documentation on \Drupal\Component\Datetime\DateTimePlus::setDefaultDateTime() for the documentation update follow-up.

Pretty sure this has introduced a random fail... see https://dispatcher.drupalci.org/job/drupal_patches/151663/

Couldn't reproduce that error in 1500x run of DateTimeWidgetTest on several PHP/SQL combos.
https://www.drupal.org/project/drupal/issues/3292008#comment-14723406

That might be because you didn't run it at the correct time of the day :)

Is it possible that we did run that _exactly_ as the day changed from one the other other? We do test a lot of timezones there, so if it runs around a full our, that seems totally possible. Especially as we use the request time of the test run process and not the current time. We can probably reduce it by changing it to getCurrentTime(), but there's still a slight chance of that happening.

That might be because you didn't run it at the correct time of the day :)

One of these days I'm going to actually use my brain...
No guarantee that will change anything to the better side of things, since apparently it hasn't been used for a long time.

Thanks @Berdir, that must indeed be the problem.

I'll try to get a test up at the (infamous) "change-of-the-day" time-of-day by tomorrow.
Anybody that can beat me to it: Please do! :)

FWIW, that's easier said than done. You need to calculate the time it takes from uploading the patch until it starts to run the test (which is not always the same as there are queues on both DrupalCI and drupal.org involved). If you run it thousands of times that helps a bit with the timeframe during which you can do that but it also depends on how the repeat option works. If that runs the test sequentially then you really will only have on run that might happen at the problematic time.

Applied patch #56 in drupal 10.0.x

@Shubham Chandra: I'm unsure why a committed patch needs a reroll?

Can this explain the failure at #3313473-26: CKEditor 5 plugin definitions should be derivable? 🤔 (@Berdir just pointed me here)