The cron settings at admin/config/system/cron need to be converted to use the new configuration system. There is currently only one setting to set the cron run time, so it should be super easy.

Tasks

  • Create a default config file names system.cron.xml and add it to the system module.
  • Convert the admin UI in system.admin.inc to read to/write from the appropriate config.
  • Convert any code that currently accesses the cron variable to use the config system.
  • Write an upgrade path from D7 -> D8 to migrate existing data to the new system and drop the appropriate variables.

This would be a good task for someone wanting to get up to speed on how the new config system works. Feel free to ping me on IRC if you need help.

Files: 
CommentFileSizeAuthor
#76 drupal8.config-cron.76.patch10.44 KBsun
PASSED: [[SimpleTest]]: [MySQL] 37,296 pass(es). View
#72 drupal8.config-cron.71.patch10.36 KBsun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.config-cron.71.patch. Unable to apply patch. See the log in the details link for more information. View
#72 interdiff.txt4.26 KBsun
#69 drupal8.config-cron.69.patch8.2 KBsun
FAILED: [[SimpleTest]]: [MySQL] 36,949 pass(es), 117 fail(s), and 0 exception(s). View
#60 drupal8.config-cron.60.patch7.32 KBsun
FAILED: [[SimpleTest]]: [MySQL] 36,594 pass(es), 90 fail(s), and 0 exception(s). View
#57 1493098-config-cmi-cron-57.patch8.07 KBjcisio
PASSED: [[SimpleTest]]: [MySQL] 36,592 pass(es). View
#53 1493098-config-cmi-cron-52.patch8.03 KBjcisio
PASSED: [[SimpleTest]]: [MySQL] 36,590 pass(es). View
#49 1493098-config-cmi-cron-49.patch8.11 KBjcisio
PASSED: [[SimpleTest]]: [MySQL] 36,511 pass(es). View
#45 1493098-config-cmi-cron-45.patch8.7 KBpcambra
PASSED: [[SimpleTest]]: [MySQL] 34,999 pass(es). View
#43 1493098-config-cmi-cron-43.patch8.89 KBjcisio
PASSED: [[SimpleTest]]: [MySQL] 35,388 pass(es). View
#42 1493098-config-cmi-cron-42.patch8.19 KBjcisio
PASSED: [[SimpleTest]]: [MySQL] 35,387 pass(es). View
#40 1493098-config-cmi-cron-40.patch8.19 KBjcisio
PASSED: [[SimpleTest]]: [MySQL] 35,391 pass(es). View
#38 1493098-config-cmi-cron-36 (1).patch8.19 KBjcisio
FAILED: [[SimpleTest]]: [MySQL] Fetch test patch: failed to retrieve [1493098-config-cmi-cron-36 (1).patch] from [drupal.org]. View
#36 1493098-config-cmi-cron-36.patch8.92 KBjcisio
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View
#36 1493098-config-cmi-cron-36-interdiff.patch2.15 KBjcisio
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1493098-config-cmi-cron-36-interdiff.patch. Unable to apply patch. See the log in the details link for more information. View
#33 1493098-cron-33.patch8.73 KBpcambra
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View
#31 1493098-cron-31.patch0 bytespcambra
PASSED: [[SimpleTest]]: [MySQL] 35,060 pass(es). View
#29 1493098-cron-29.patch8.16 KBpcambra
FAILED: [[SimpleTest]]: [MySQL] 35,914 pass(es), 1 fail(s), and 0 exception(s). View
#22 1493098-21-cron.patch8.62 KBRok Žlender
PASSED: [[SimpleTest]]: [MySQL] 35,677 pass(es). View
#21 1493098-20.cron_.patch8.18 KBRok Žlender
PASSED: [[SimpleTest]]: [MySQL] 35,692 pass(es). View
#18 1493098-18.cron_.patch8.18 KBRok Žlender
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View
#17 1493098-17-cron.patch8.62 KBbeejeebus
PASSED: [[SimpleTest]]: [MySQL] 35,686 pass(es). View
#15 issue_1493098-15-cron.patch8.01 KBwamilton
FAILED: [[SimpleTest]]: [MySQL] 35,672 pass(es), 1 fail(s), and 0 exception(s). View
#13 issue_1493098-11-cron.patch7.57 KBwamilton
FAILED: [[SimpleTest]]: [MySQL] 35,609 pass(es), 10 fail(s), and 4 exception(s). View
#9 issue_1493098-9-cron.patch7.48 KBwamilton
FAILED: [[SimpleTest]]: [MySQL] 35,616 pass(es), 10 fail(s), and 0 exception(s). View
#5 1493098-5-cron.patch2.98 KBPol
FAILED: [[SimpleTest]]: [MySQL] 35,678 pass(es), 1 fail(s), and 0 exception(s). View
#4 1493098-4-cron.patch2.45 KBPol
FAILED: [[SimpleTest]]: [MySQL] 35,676 pass(es), 1 fail(s), and 0 exception(s). View
#1 1493098-1-cron.patch2.74 KBPol
FAILED: [[SimpleTest]]: [MySQL] 35,688 pass(es), 1 fail(s), and 0 exception(s). View

Comments

Pol’s picture

Status: Active » Needs review
FileSize
2.74 KB
FAILED: [[SimpleTest]]: [MySQL] 35,688 pass(es), 1 fail(s), and 0 exception(s). View

First shot of the patch.

beejeebus’s picture

this looks good to me. the only remaining bit is the upgrade path.

Status: Needs review » Needs work

The last submitted patch, 1493098-1-cron.patch, failed testing.

Pol’s picture

FileSize
2.45 KB
FAILED: [[SimpleTest]]: [MySQL] 35,676 pass(es), 1 fail(s), and 0 exception(s). View

Updated patch, fix the typo (cron_max_threshold <> cron_safe_threshold)

The tests will fail because of #1324618: Implement automated config saving for simple settings forms.
Once this issue will be fixed, the test will be successful.

Pol’s picture

FileSize
2.98 KB
FAILED: [[SimpleTest]]: [MySQL] 35,678 pass(es), 1 fail(s), and 0 exception(s). View

Added the upgrade path in system.install.
The upgrade path is a simple variable_del('cron_safe_threshold');.

beejeebus’s picture

Status: Needs work » Needs review

just setting to needs review so the bot will see it.

Status: Needs review » Needs work

The last submitted patch, 1493098-5-cron.patch, failed testing.

beejeebus’s picture

ok, i spent a bit more time looking at this. there are a bunch more cron_* variables to convert to the new system.

from a bit of grepping, i can see:

cron_last
cron_threshold_warning
cron_threshold_error
cron_key

there is also cron_semaphore, but i think that should go into the key/value store when that's ready.

wamilton’s picture

Status: Needs work » Needs review
FileSize
7.48 KB
FAILED: [[SimpleTest]]: [MySQL] 35,616 pass(es), 10 fail(s), and 0 exception(s). View

Here's my stab at just that. cron_last is also 'state' not 'config' just like cron_semaphore, so no changes there. Also helped out aggregator and simpletest.

Status: Needs review » Needs work

The last submitted patch, issue_1493098-9-cron.patch, failed testing.

beejeebus’s picture

Status: Needs work » Needs review

#9 looks good. i've created a 1493098-cron branch for this code, and will probably push #9 into it if the tests come back green.

beejeebus’s picture

oh, doh. ok i'm going to look into that error.

also, wamilton, if you're at drupalcon, come find me! i'm beejeebus in #durpalcon and #drupal-contribute.

wamilton’s picture

FileSize
7.57 KB
FAILED: [[SimpleTest]]: [MySQL] 35,609 pass(es), 10 fail(s), and 4 exception(s). View

My bad, calling ::save() now and not trying to get the variable names from config()->get(). I guess we hardcode instead?

Status: Needs review » Needs work

The last submitted patch, issue_1493098-11-cron.patch, failed testing.

wamilton’s picture

Status: Needs work » Needs review
FileSize
8.01 KB
FAILED: [[SimpleTest]]: [MySQL] 35,672 pass(es), 1 fail(s), and 0 exception(s). View

Holy cow, lets get our files into the diff!

Per beejeebus, we are still trying to infer variable names from the config() active store as they really ought to be present.

@beejeebus: thanks for the help!

Status: Needs review » Needs work

The last submitted patch, issue_1493098-15-cron.patch, failed testing.

beejeebus’s picture

Status: Needs work » Needs review
FileSize
8.62 KB
PASSED: [[SimpleTest]]: [MySQL] 35,686 pass(es). View

ok, two things to fix here:

- we're not calling save in the test

- we're need to use the $form_state['conf'] hack in system_settings_form_submit().

hopefully this will pass.

Rok Žlender’s picture

Status: Needs review » Needs work
FileSize
8.18 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View

Few things in patch #17

+++ b/core/modules/system/system.installundefined
@@ -283,7 +283,8 @@ function system_requirements($phase) {
+  ¶

Extra line

+++ b/core/modules/system/system.installundefined
@@ -515,7 +516,7 @@ function system_install() {
+  config('system.cron')->set('cron_key', $cron_key);

Is there a reason why save() is not called in system_install for setting cron_key

Attached patch fixes these two.

Rok Žlender’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 1493098-18.cron_.patch, failed testing.

Rok Žlender’s picture

Status: Needs work » Needs review
FileSize
8.18 KB
PASSED: [[SimpleTest]]: [MySQL] 35,692 pass(es). View

Now rerolled with update hook.

Rok Žlender’s picture

FileSize
8.62 KB
PASSED: [[SimpleTest]]: [MySQL] 35,677 pass(es). View

Ok and now with added new file that is missing in my previous patches :/

Mark Theunissen’s picture

Status: Needs review » Needs work

This is my first look at the config management system.

+++ b/core/modules/system/system.installundefined
@@ -1762,6 +1764,28 @@ function system_update_8005() {
+function system_update_8006() {
+  $var_names = array_keys(config('system.cron')->get());

$var_names will be empty, as there's nothing in the active store at this point. Therefore this update does nothing - it's a circular problem - this update is supposed to populate the active store but can't as it relies on the existence of the store to run. Perhaps get the names from somewhere else or hardcode them?

+++ b/core/modules/system/system.installundefined
@@ -1762,6 +1764,28 @@ function system_update_8005() {
+      ->fields('v')

Rather specify the exact fields you would like to query.

+++ b/core/modules/system/system.installundefined
@@ -1762,6 +1764,28 @@ function system_update_8005() {
+    foreach($var_values as $name => $val) {

Minor code style, put a space after 'foreach'

beejeebus’s picture

Status: Needs work » Reviewed & tested by the community

ok, this looks good to go to me.

Mark Theunissen’s picture

Status: Reviewed & tested by the community » Needs work

The update didn't work for me when I tested it, see above.

Lars Toomre’s picture

This patch has the added issue that the variables are first deleted and then saved to new config file. The order needs to be reversed to prevent possible data loss if the write to config fails.

pcambra’s picture

Status: Needs work » Postponed
heyrocker’s picture

Status: Postponed » Needs work

So this needs to be rerolled to use update_variables_to_config() now that that has gotten committed.

pcambra’s picture

Status: Needs work » Needs review
FileSize
8.16 KB
FAILED: [[SimpleTest]]: [MySQL] 35,914 pass(es), 1 fail(s), and 0 exception(s). View

Rerolling it using the new update function.

Status: Needs review » Needs work

The last submitted patch, 1493098-cron-29.patch, failed testing.

pcambra’s picture

FileSize
0 bytes
PASSED: [[SimpleTest]]: [MySQL] 35,060 pass(es). View

Another try.

Changes in here:

Not sure what's the plan for 'state' vars such as cron_last.

Lars Toomre’s picture

Zero byte patch in #31.

pcambra’s picture

Status: Needs work » Needs review
FileSize
8.73 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View

Oops, not sure what happened. Attaching a new one :)

swentel’s picture

Status: Needs review » Needs work
+++ b/core/modules/system/system.admin.incundefined
@@ -1588,11 +1588,29 @@ function system_cron_settings() {
+    'name' => 'system.cron',
+    'path' => 'cron_safe_threshold',
+  );
+ ¶

Whitespace

-29 days to next Drupal core point release.

jcisio’s picture

Assigned: Unassigned » jcisio

I'm trying to work on this today.

jcisio’s picture

Status: Needs work » Needs review
FileSize
2.15 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1493098-config-cmi-cron-36-interdiff.patch. Unable to apply patch. See the log in the details link for more information. View
8.92 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View

Fix a fatal error, use $config when available and do not create a new $config when not necessary (also, for the consistency).

Status: Needs review » Needs work

The last submitted patch, 1493098-config-cmi-cron-36-interdiff.patch, failed testing.

jcisio’s picture

Status: Needs work » Needs review
FileSize
8.19 KB
FAILED: [[SimpleTest]]: [MySQL] Fetch test patch: failed to retrieve [1493098-config-cmi-cron-36 (1).patch] from [drupal.org]. View

Don't know why some change was included in interdiff but not in the full patch.

Status: Needs review » Needs work

The last submitted patch, 1493098-config-cmi-cron-36 (1).patch, failed testing.

jcisio’s picture

Status: Needs work » Needs review
FileSize
8.19 KB
PASSED: [[SimpleTest]]: [MySQL] 35,391 pass(es). View

Space in filename?

heyrocker’s picture

Status: Needs review » Needs work

This patch is missing the system.cron.xml file which provides defaults for the cron settings. This file is in earlier patches. I'm not really sure why all tests pass given this, that is something worth looking at.

jcisio’s picture

Status: Needs work » Needs review
FileSize
8.19 KB
PASSED: [[SimpleTest]]: [MySQL] 35,387 pass(es). View

system.cron.xml added.

jcisio’s picture

FileSize
8.89 KB
PASSED: [[SimpleTest]]: [MySQL] 35,388 pass(es). View

Sorry, this time it is really.

heyrocker’s picture

Status: Needs review » Needs work

Unfortunately this now needs a reroll because system_update_8008() is already declared, so it needs to be renamed to system_update_8009(). However other than that, I've tested everything and it appears ready to go. Make that change and I'll RTBC. Thanks!

pcambra’s picture

Status: Needs work » Needs review
FileSize
8.7 KB
PASSED: [[SimpleTest]]: [MySQL] 34,999 pass(es). View

Updated and back to CNR

heyrocker’s picture

Status: Needs review » Reviewed & tested by the community

Awesome!

catch’s picture

Status: Reviewed & tested by the community » Needs work

Sorry folks this will need a re-roll for the cron.php hunk, since that file no longer exists.

jcisio’s picture

I'm working for a patch.

jcisio’s picture

Status: Needs work » Needs review
FileSize
8.11 KB
PASSED: [[SimpleTest]]: [MySQL] 36,511 pass(es). View

Here you are.

heyrocker’s picture

Status: Needs review » Reviewed & tested by the community

Thanks jcisio!

tstoeckler’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/system/config/system.cron.xml
@@ -0,0 +1,8 @@
+  <cron_threshold_error>1209600</cron_threshold_error>
+<cron_key>drupal</cron_key>

Seems like the indentation is off.

+++ b/core/modules/system/system.admin.inc
@@ -1589,11 +1589,29 @@ function system_cron_settings() {
+  $form_state['config']['cron_safe_threshold'] = array(
+    'name' => 'system.cron',
+    'path' => 'cron_safe_threshold',
+  );

I think it's weird that we're adding this code, even though system_settings_form() has not been converted yet. Maybe add a little comment about that?

Needs work for the first at least, the second is debatable I guess.

jcisio’s picture

Status: Needs work » Needs review

Same patch as #49, solves two comments in #51. I just added a simple @todo, there are a few debatable points, but they could be fixed later. A more important point, which will require an update function, is how the variables are name: here in system.cron.xml they are named with cron_ prefix, but in other (e.g. search.module), the variable prefix was removed. But I think it is bikeshedding, as 51 comments and nobody points it out, so I leave it as is.

jcisio’s picture

FileSize
8.03 KB
PASSED: [[SimpleTest]]: [MySQL] 36,590 pass(es). View

Forgot the patch.

tstoeckler’s picture

Actually, #53 is a valid point. The config keys should not be prefixed with cron.
I guess?!
Current examples in core:
http://drupalcode.org/project/drupal.git/blob/HEAD:/core/modules/system/...

http://drupalcode.org/project/drupal.git/blob/HEAD:/core/modules/block/c... (Hmm... This one uses "block_cache" as key in "block.performance")

http://drupalcode.org/project/drupal.git/blob/HEAD:/core/modules/image/c...
and friends

heyrocker’s picture

Status: Needs review » Reviewed & tested by the community

I would like to keep the keys the same as their respective variables in Drupal 7. I pushed the search settings patch back to needs work based on this. If we decide to change the names, we can do it in a followup patch.

Therefore I'm back to RTBC on this.

catch’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Novice

This no longer applies after the simpletest PSR-0 conversion. Tagging novice for re-roll.

jcisio’s picture

Status: Needs work » Needs review
FileSize
8.07 KB
PASSED: [[SimpleTest]]: [MySQL] 36,592 pass(es). View

Git detected file move and created the attached patch automatically. It should hopefully work.

catch’s picture

Status: Needs review » Reviewed & tested by the community
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks all.

sun’s picture

Status: Fixed » Needs review
FileSize
7.32 KB
FAILED: [[SimpleTest]]: [MySQL] 36,594 pass(es), 90 fail(s), and 0 exception(s). View

Thanks for your hard work on this.

Re-opening due to a couple of issues though, and to align the new names with #1599554: Tutorial/guidelines for how to convert variables into configuration

+++ b/core/modules/system/config/system.cron.xml
@@ -0,0 +1,8 @@
+  <cron_max_threshold>10800</cron_max_threshold>

This variable does not exist.

+++ b/core/modules/system/system.admin.inc
@@ -1589,11 +1589,30 @@ function system_cron_settings() {
   $form['cron']['cron_safe_threshold'] = array(
     '#type' => 'select',
     '#title' => t('Run cron every'),
-    '#default_value' => variable_get('cron_safe_threshold', DRUPAL_CRON_DEFAULT_THRESHOLD),
+    '#default_value' => config('system.cron')->get('cron_safe_threshold'),
     '#options' => array(0 => t('Never')) + drupal_map_assoc(array(3600, 10800, 21600, 43200, 86400, 604800), 'format_interval'),
   );

Grepping for this name did not yield any other writes than this configuration form.

Further research showed that this is completely broken and the UI exposes an entirely bogus setting currently. The relevant code logic only differs between 0 or >0, and the actual interval is not taken into account at all.

Fixing that properly is left to #1554872: "Run cron every" setting (cron_safe_threshold) is not an interval but a Boolean

For this issue, we just want to have a sane config key.

+++ b/core/modules/system/system.admin.inc
@@ -1589,11 +1589,30 @@ function system_cron_settings() {
+  // @todo This needs to be reviewed when #1324618 gets in.
+  $form_state['config']['cron_safe_threshold'] = array(
+    'name' => 'system.cron',
+    'path' => 'cron_safe_threshold',
+  );

This is not used anywhere.

sun’s picture

Assigned: jcisio » sun
catch’s picture

What is 'web'?

Status: Needs review » Needs work

The last submitted patch, drupal8.config-cron.60.patch, failed testing.

sun’s picture

Status: Needs work » Needs review

"threshold.poormanscron" would be most appropriate and most understandable for long-time Drupalers ;)

For anyone else + newcomers? I've no idea! How do you call the frequency or time interval for calling a function that is called as a PHP shutdown function in order to run Drupal cron jobs, mainly via web browsers of random site visitors? :)

"threshold.web" in the sense of "the threshold/interval to execute the web-based/-triggered cron crunner" made some sense to me.

sun’s picture

Checking the test failures, I'm fairly sure this patch will pass as-is, after #1496542: Convert site information to config system has landed.

tstoeckler’s picture

Status: Needs review » Needs work
+++ b/core/modules/system/system.admin.inc
@@ -1579,26 +1579,24 @@ function system_cron_settings($form, &$form_state) {
-  // @todo This needs to be reviewed when #1324618 gets in.
-  $form_state['config']['cron_safe_threshold'] = array(
-    'name' => 'system.cron',
-    'path' => 'cron_safe_threshold',
-  );

Above it was (sort of) discussed, that we put this there so that #1324618: Implement automated config saving for simple settings forms automatically works with this patch. I wasn't a huge advocate of putting it in, just saying it was put there for a reason.

+++ b/core/modules/system/system.admin.inc
@@ -1579,26 +1579,24 @@ function system_cron_settings($form, &$form_state) {
+  $form = system_settings_form($form);
+  $form['#submit'] = array('system_cron_settings_submit');

Can you explain what the point of this is? I don't get it.

Regarding the key name, maybe "auto_run", as after that threshold cron is run "automatically" (from the viewpoint of the site administrator)?

tstoeckler’s picture

Status: Needs work » Needs review

Sorry, leaving at "needs review" for now.

sun’s picture

  1. None of these conversion patches should contain any premature code for #1324618: Implement automated config saving for simple settings forms yet. Regardless of what the final solution will be, it will definitely look different than the removed lines here.
  2. The most simple way to convert settings forms at this point is to retain usage of system_settings_form() to get a consistent actions container and submit button, but override the #submit handler being set by it, in order to prevent any variables from getting written. We likely want to add that to #1599554: Tutorial/guidelines for how to convert variables into configuration
  3. config('system.cron')->get('threshold.autorun') makes sense to me. Wanna re-roll with that?

    That said, depending on its outcome, it's possible that #1554872: "Run cron every" setting (cron_safe_threshold) is not an interval but a Boolean will likely remove the 'threshold.' prefix, but that's none of our business here.

New patches will still fail as long as #1496542: Convert site information to config system did not land.

sun’s picture

FileSize
8.2 KB
FAILED: [[SimpleTest]]: [MySQL] 36,949 pass(es), 117 fail(s), and 0 exception(s). View

Re-rolled with adjustments against latest HEAD.

Status: Needs review » Needs work

The last submitted patch, drupal8.config-cron.69.patch, failed testing.

jcisio’s picture

cron_max_threshold is lost?

sun’s picture

Status: Needs work » Needs review
FileSize
4.26 KB
10.36 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.config-cron.71.patch. Unable to apply patch. See the log in the details link for more information. View

Regarding cron_max_threshold, see #60 - the variable is entirely obsolete.

Apparently, the Drupal installer does not install system module in the regular/proper procedure. Due to that, the system.cron:key is never actually set/updated after the Drupal installation. Attached patch additionally fixes the Drupal installer to install System module correctly.

sun’s picture

#72: drupal8.config-cron.71.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Novice, +Configuration system, +Config novice

The last submitted patch, drupal8.config-cron.71.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
10.44 KB
PASSED: [[SimpleTest]]: [MySQL] 37,296 pass(es). View

Re-rolled against HEAD.

cosmicdreams’s picture

Status: Needs review » Reviewed & tested by the community

great patch, looks like it catches everything. Only spacing issues here that I'm quibbling about. IMO it's RTBC

+++ b/core/modules/system/system.admin.incundefined
@@ -1596,40 +1595,34 @@ function system_cron_settings($form, &$form_state) {
+
   $status = '<p>' . t('Last run: %cron-last ago.', array('%cron-last' => format_interval(REQUEST_TIME - variable_get('cron_last')),)) . '</p>';

spacing here seems to hurt not help readibility

+++ b/core/modules/system/system.admin.incundefined
@@ -1596,40 +1595,34 @@ function system_cron_settings($form, &$form_state) {
+
   $form['cron'] = array(

as well as here

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Looks good to me. Committed to 8.x. Thanks!

sun’s picture

Issue tags: +API change

Automatically closed -- issue fixed for 2 weeks with no activity.