🇯🇵Japan @tom konda

Update title of this issue.

OK. I notice to create a meta issue next time like this type of issue.

Added change record. ( https://www.drupal.org/node/3525136 → )
Please review.

tom konda → created an issue.

In this issue, there are two feature branches.
The 3515975-phpcs-issues is a target branch for currently opened MR 29.
The autosave_form-3515975-3515975-phpcs-issues has the latest commit 80bb6c26.
Which is correct branch?

tom konda → made their first commit to this issue’s fork.

tom konda → created an issue.

Added the reason why jQuery UI URL is removed to issue summary.
Please review.

I fixed PHPCS errors.
Please review.

tom konda → made their first commit to this issue’s fork.

I added configuration schema and changed max level label to Unlimited.
But, SitemapUpdateSettingsTest::testUpdateHookN() is still failing.
I think probably need to implement hook_update_N() code.

When this issue was created, the commit id 8c33fb4f was already committed but the commit id ce26a90e was not committed for the feature branch.
I rebased the feature branch and all tests ware passed.

@mparker17 Sorry for my late reply.
Thank you for your reviewing.
I'll try to add configuration schema and change unlimited value.

tom konda → created an issue.

I checked both of Intersection Observer and scrollTo options argument in MDN and "Can I use".
I think all of browsers that require to support for Drupal, work well.
If we need to checking actual browser behavior, need to further working.

On the MDN, both of features display baseline badge as "Widely available".
Both of features had been supported at least since 2020 and compatibility table shows all of browsers are supported so all of supported browsers that Drupal have can work well.
On the "Can I use", all of the latest 2 major versions browsers that Drupal need to support, are shown as green both of Intersection Observer and scrollTo options argument.

Note:

1. The latest release of Firefox ESR on 2025-04-01 works as well as Firefox 128. (Firefox 128 was released in 2024)
2. Opera Mini (except for 'extreme data savings' mode) works as well as Opera Mobile. (See #3202818: Remove untrue declaration of Opera Mini (in reverse proxy mode) support from Drupal 9.3 → )

> - make sure there are no failures in phpcs, cspell, eslint, and stylelint. Make the job fail if there are future failures

All validate stage jobs are green now.

I rebased feature branch and added a test for menu depth option.
But, need to fix SitemapUpdateSettingsTest::testUpdateHookN.

tom konda → made their first commit to this issue’s fork.

Issue #3176383 is removed.
IE11 had been dropped support since Drupal 10. No need to add polyfill.

In my opinion, other instances cannot be replaced.

I added type checking for a JavaScript theme function.
Please review.

tom konda → created an issue.

tom konda → created an issue.

@avpaderno Oh, sorry. I misunderstand that MR!6 is valid branch.

I fixed MR !6 and all validate stage was all green.
Please review.

tom konda → created an issue.

tom konda → made their first commit to this issue’s fork.

tom konda → made their first commit to this issue’s fork.

tom konda → made their first commit to this issue’s fork.

tom konda → made their first commit to this issue’s fork.

tom konda → made their first commit to this issue’s fork.

tom konda → made their first commit to this issue’s fork.

I think this issue is already solved in 🐛 Drupal coding standards Needs work .

I fix all of ESLint errors and all of validate stages are green.
Please review.

tom konda → made their first commit to this issue’s fork.

tom konda → created an issue.

Add testing code for template literal.
Please review.

I reran Functional JavaScript tests and all tests are passed.
It is probably random failure.
Please review.

Ran Functional JavaScript tests several times.
But there is an error on Drupal\Tests\ckeditor5\FunctionalJavascript\ImageUrlProviderTest.
Need to resolve this error.

tom konda → created an issue.

tom konda → created an issue.

Fix ESLint probrem.
Please review.

tom konda → created an issue.

I resolved merge conflict and stylelint is green.
Please review.

tom konda → made their first commit to this issue’s fork.

I rebased MR and fixed all of ESLint errors for latest release.
ESLint is green now. Please review.

tom konda → made their first commit to this issue’s fork.

I rebased the issue branch and fixed some of validate stage errors.

tom konda → made their first commit to this issue’s fork.

I rebased the issue branch and merge error is resolved.
Please review.

I rebased the issue branch and fixed no-var errors.
There are following ESLint probrems left.

• no-undef
• no-new-func
• no-restricted-syntax

Fix ESLint error and rebuild CKEditor5 plugin JavaScript file.

I fixed phpcs errors and phpcs tests are all green.

tom konda → made their first commit to this issue’s fork.

tom konda → created an issue.

I fixed following ESLint errors. But still there is ESLint errors, so need to work.

• ESLint fixable errors
• Camelcase errors
• Eqeqeq errors
• A no-restricted-globals error
• Errors after line 278 except no-undef error

tom konda → made their first commit to this issue’s fork.

Fix pipeline ESLint errors.
Please review.

tom konda → made their first commit to this issue’s fork.

tom konda → created an issue.

tom konda → created an issue.

tom konda → made their first commit to this issue’s fork.

tom konda → created an issue.

tom konda → created an issue.

I investigated a FunctionalJavaScriptTests behavior with the drupalci/chromedriver:dev container and found set wrong value to event.key and event.code on my local environment.
Behat\Mink\Element\NodeElement::keyDown() only calls driver's keyDown() method, so may have a problem on WebDriver or other.

Investigation method

1. Change /core/misc/tabledrag.js to record some KeyboardEvent value to drupalSetting on a keydown event. (Code 1)
2. Change /core/tests/Drupal/FunctionalJavascriptTests/TableDrag/TableDragTest.php to output drupalSettings.keyTest value. (Code 2)
3. Test /core/tests/Drupal/FunctionalJavascriptTests/TableDrag/TableDragTest.php

Code 1

    // On blur, fire the same function as a touchend/mouseup. This is used to
    // update values after a row has been moved through the keyboard support.
    $handle.on('blur', (event) => {
      if (self.rowObject && self.safeBlur) {
        self.dropRow(event, self);
      }
    });

    // Add arrow-key support to the handle.
    $handle.on('keydown', (event) => {
      // I added below code for investigation.
      const {charCode, keyCode, code, key} = event;
      drupalSettings.keyTest = {
        charCode, keyCode, code, key
      };
      // I added above code for investigation.

Code 2

  protected function moveRowWithKeyboard(NodeElement $row, $arrow, $repeat = 1): void {
    $keys = [
      'left' => 37,
      'right' => 39,
      'up' => 38,
      'down' => 40,
    ];
    if (!isset($keys[$arrow])) {
      throw new \InvalidArgumentException('The arrow parameter must be one of "left", "right", "up" or "down".');
    }

    $key = $keys[$arrow];

    $handle = $row->find('css', 'a.tabledrag-handle');
    $handle->focus();

    for ($i = 0; $i < $repeat; $i++) {
      $this->markRowHandleForDragging($handle);
      $handle->keyDown($key);
      // I added below code for investigation.
      print_r($this->getDrupalSettings()['keyTest']);
      // I added above code for investigation.
      $handle->keyUp($key);
      $this->waitUntilDraggingCompleted($handle);
    }

    $handle->blur();
  }

Result

PHPUnit 10.5.38 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.13
Configuration: /opt/drupal/web/core/phpunit.xml.dist

..Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
FArray
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
Array
(
    [charCode] => 40
    [code] => 
    [key] => (
    [keyCode] => 40
)
Array
(
    [charCode] => 37
    [code] => 
    [key] => %
    [keyCode] => 37
)
FArray
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
Array
(
    [charCode] => 40
    [code] => 
    [key] => (
    [keyCode] => 40
)
Array
(
    [charCode] => 37
    [code] => 
    [key] => %
    [keyCode] => 37
)
Array
(
    [charCode] => 40
    [code] => 
    [key] => (
    [keyCode] => 40
)
Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
FArray
(
    [charCode] => 40
    [code] => 
    [key] => (
    [keyCode] => 40
)
Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)
F                                                              6 / 6 (100%)Array
(
    [charCode] => 39
    [code] => 
    [key] => '
    [keyCode] => 39
)

/core/modules/locale/js/locale.admin.js uses event.code instead of event.key because event.key when space key is pressed, returns ' ' (a white space character) which make it little difficult code reading.

tom konda → created an issue.

> let's create an issue for the unicorn plugin
I created an issue for the unicorn plugin.
📌 Consider to use some of plugin:unicorn/recommended ESLint rules for Drupal core Active