🇮🇳India @keshav patel

keshav patel → created an issue.

Hello /u/alexbukach,

Can you please check this, It'll be better if it is merged.

Hello @avpaderno → , Thanks for a thorough review!
I've worked on suggested improvements, please check.

Applied suggested changes.

PHPUnit tests are failing, check here.

Requesting for the "maintainer" role for the guide:

I've worked on Linux, WSL2 and XAMPP on Windows and also Docker on WSL2 Drupal Setups.

We can mention the steps with details about these setups, also on windows there are some pre-requisites and adjustments that can be added on this guide to make it more helpful.

@xen → ,

I've added 📌 Update README file and projects description as per change introduced on #3505292 Active to cover the documentation part related to this task.

keshav patel → created an issue.

Got it @sandip → .

keshav patel → made their first commit to this issue’s fork.

Reviewed the MR, the removed part is not added back for stable9.

Yes I checked that, we don't need existence check after unsetting the hook. I've updated the comment to be make it informative and squashed all the work in a single commit.

Moving to RTBC.

I think we're good now, just needs a review if anything is left.

Also, once this is merged. I think I'll be better to specify:

Keep URL alias field untranslated.

as requirement.

@xen → Till now we're done with the duplicate alias issue.

Now we're left with, multiple aliases existence for the same entity i.e. same alias not getting updated.

Regarding:

What do you mean specifically? I notice that path_entity_base_field_info() sets the pathbase field to translatable, but I don't know if that really affect us as we're dealing with alias.

path_entity_translation_create() seems to be the cause of our woes, as it unsets the pid of the alias (but keep the alias) when creating a translation. Maybe disabling that will make things behave.

Yes, the hook path_entity_translation_create() is the culprit here, line: $translation->get($field_name)->pid = NULL; is to be taken of.

Taking your suggestion, I removed the form alter implementation so that the Alias can be added / updated anywhere.

With that, we're left with two test cases:

URL Alias is: Translated and $translation->get($field_name)->pid = NULL; is commented

-> When node is created (let's say alias: '/test') and on translation add, alias is changed (let's say to 'test-fr'), the alias does not get updated and the alias still remains as '/test'.
-> That is, during create operation [translation add] the changed alias value is not saved even if it's updated.
-> When translated are created. after that if the alias is updated on source or translation . that changes happen correctly and alias are reflected.

URL Alias is: Un-translated and $translation->get($field_name)->pid = NULL; is commented

-> Only one alias instance is present and is getting updated. [On Node create / edit, translation add / update]

Considering this, we've to keep the URL alias as un-translated and Override path_entity_translation_create() hook.

Thanks for the review @vishal.kadam → .

Also, as mentioned on What to expect from the review process → section:

Once all issues have been addressed:
The reviewers will change the status of the issue to Reviewed & tested by the community

Please change the status to RTBC.

Thanks for the feedback @vishalkadam → .

Fixed all reported issues, please review.

I've gone through Apply for permission to opt into security advisory coverage → page to understand all the requirements.

Changing status to "Needs review".

keshav patel → created an issue.

Hello @xen → , thanks for the feedback!

Yes will update the MR to respect OOP.

But the second part:

hiding the path on translations hides the fact that the translation *does* have an alias. I'd rather skip that part and live with the fact that editing the path on the translation also edits the original language alias.

I hid that field from translation pages by considering the scenarios duplicate aliases, multiple aliases and when using the "pathauto" module as well, like there are some points to take into consideration:
1) The URL Alias field must set as untranslated when using this module.
2) If kept translated this module can create duplicates or multiple aliases(distinct) for same "path".
3) In case when "pathauto" exists and "Generate Automatic Alias" is checked, it'll create duplicates.

I even tried to overcome the duplicate / multiple alias generation issue while keeping the URL alias field visible everywhere, but alias entity is generated before it reaches "NeutralPathAliasStorage.php".

Also, please do let me know if anything else I can try here..

The MR contains 1000+ changes which are outside of this issues scope.
Please raise correct MR.

Raised an MR with the provided patch on #4#4,
Keeping it on Needs work until tests are added.

keshav patel → made their first commit to this issue’s fork.

Created an MR using #2 patch, needs review.

keshav patel → made their first commit to this issue’s fork.

Created an MR from the patch provided on #2, keeping it on needs review.

keshav patel → made their first commit to this issue’s fork.

@swentel →
Please check that "file_validate_extensions" is mistakenly substituted by "FileImageDimensions", it should be changed to "FileExtension".

Please describe the issue in accordance with the default provided issue summary template, It'll become more readable and helpful.

Thanks!

MR contains 136 changes, also the target branch for the MR is incorrect.

It should be raised against 3.x branch.

Changes looks good to me as per the change record → , still marking as "Needs review" for others to review.

Fixed some Drupal coding standard issues, but I think there are 4 such issue which we've to live with or we can suppress them:

FILE: src/Plugin/CKEditor4To5Upgrade/AdvancedLink.php
-----------------------------------------------------------------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
-----------------------------------------------------------------------------------------------------------------------------
 30 | ERROR | Public method name "AdvancedLink::mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem" is not in lowerCamel format
 37 | ERROR | Public method name "AdvancedLink::mapCKEditor4SettingsToCKEditor5Configuration" is not in lowerCamel format
 44 | ERROR | Public method name "AdvancedLink::computeCKEditor5PluginSubsetConfiguration" is not in lowerCamel format
-----------------------------------------------------------------------------------------------------------------------------
Reason: Core too uses naming in the same format. (@see Drupal\ckeditor5\Plugin\CKEditor4To5UpgradePluginInterface)

FILE: /tests/src/Kernel/CKEditor4To5Upgrade/UpgradePathCompletenessTest.php
------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------
 15 | ERROR | Class name doesn't match filename; expected "class UpgradePathCompletenessTest"
------------------------------------------------------------------------------------------------------------
Reason: Class exists with the same name (@see line:42), but there are multiple classes on the same file.

keshav patel → made their first commit to this issue’s fork.

@webmestre → Please test your installed module version quickly in fresh Drupal instance here: simplytest.me.

I think you're facing site specific issues due to customizations. In case you're able to replicate the issue on simplytest.me, please attach the screenshots.

Please check out this module: AI Image Alt Text → , maybe it can help.

@webmestre →

Please add few more details, I was not able to reproduce the issue with this minimal information.
Also, when I picked the date ranges.. the time picker needed to be clicked in order to record the changes (means it was reflecting correct results, ex: 2026-08-05 12:00)

The issue status had to be on Needs review.

Tested the the MR changes, no phpcs issues found.
Moving it to RTBC.

Rerolled #31 to MR, fixed coding standards as well.

But it's failing two phpstan testcases.

keshav patel → made their first commit to this issue’s fork.

Thanks a lot for saving debugging time.

Works fine.

Thanks everyone, I've merged the changes.

Minor correction for #3
Make URL Alias field, accessible only on source language. handles the issue-2 for us.
Marking issue as "Needs review".

Issue-1 is handled and needs review , Issue-2 remains..
Keeping issue on "Needs work".

keshav patel → created an issue.

Although purpose is clear from the issue title, still the issue summary needs to be updated.

In the menu, the " Local development guide → " and " Installing Drupal → => Install Drupal using DDEV → " Both contain the steps to get started with drupal using ddev, but going through the " Install Drupal using DDEV → " page, I found it better descriptive.

Suggestion:
We can merge both pages and bring Install Drupal using DDEV → link at the primary level (replace at "Local development guide" menu item. )

correction in issue summary.

keshav patel → created an issue.

Update the code to resolve "included" key data correctly for multilingual sites, please review.

keshav patel → created an issue.

@ressa as per #17 I guess you're referring to hook_install() and hook_uninstall().
yes we can do that, but standard settings will vary user to user, like for live site "aggregation can be kept ON and debug mode disabled".

Can you please share what settings you want to default it to upon module uninstall ? will share that part as well..

@ressa you can use this code snippet on any hook, event or on devel_php executer. It'll work.
Then you can visit "/admin/config/development/performance" and "/admin/config/development/settings" to verify the changes, no extra code needed.

keshav patel → created an issue.

Read above comments, and thought of trying it myself to achieve the listed requirement via single flag on Settings.php file.

Settings.php: [Set caching_aggregation flag to FALSE]

$settings['caching_aggregation'] = FALSE;

Code:

    $caching_aggregation = \Drupal\Core\Site\Settings::get('caching_aggregation', TRUE);

    if(!$caching_aggregation){
      // Enable Twig Debug Mode.
      $keyValueFactory = \Drupal::service('keyvalue');
      $development_settings = $keyValueFactory->get('development_settings');
      $twig_development = [
        'disable_rendered_output_cache_bins' => TRUE,
        'twig_debug' => TRUE,
        'twig_cache_disable' => TRUE,
      ];
      $development_settings->setMultiple($twig_development);

      // Disable CSS and JS Aggregation.
      $performance = \Drupal::configFactory()->getEditable('system.performance');
      $performance->set('css.preprocess',FALSE);
      $performance->set('js.preprocess',FALSE);
      $performance->save();
    }

But the question still remains that, if not drush command then when to invoke it?
For testing purposes I invoked it on form submit event.

Hope it helps someone, Thanks!

@isalmanhaider, can you be more specific about which Drupal version you faced the issue? I've tested this on Drupal 10.2, and the module was uninstalled without any problems.

Adding "issue summary update" tag as well.

Updated the #8 patch, the patch on #10 applies cleanly. please review.

Keshav Patel → made their first commit to this issue’s fork.

Keshav Patel → made their first commit to this issue’s fork.

Keshav Patel → made their first commit to this issue’s fork.

Keshav Patel → made their first commit to this issue’s fork.

Updated as per #5, please review.

Keshav Patel → made their first commit to this issue’s fork.

Fixed the typo, please review.

Keshav Patel → made their first commit to this issue’s fork.

Updated as per #21, please review.

Keshav Patel → made their first commit to this issue’s fork.

Added the issue summary according to the standard issue template.

Keshav Patel → made their first commit to this issue’s fork.

Keshav Patel → made their first commit to this issue’s fork.

Please mention the steps to reproduce the issue and update the issue summary accordingly.

Keshav Patel → made their first commit to this issue’s fork.

Created the MR using the #26 patch and addressed comment #35 as well, but the pipeline failed. keeping it on "Needs work".., please review the changes.

Change as per #35:

   /**
    * Returns the filter format this text editor is associated with.
    *
    * @return \Drupal\filter\FilterFormatInterface
    *
    * @throws \DomainException
 -  *   If called without an associated filter format.
 +  *   Thrown if the text editor is called without an associated filter format.
    */
  public function getFilterFormat();

Keshav Patel → made their first commit to this issue’s fork.

Patch Rerolled and all checks passed. please review.

Keshav Patel → made their first commit to this issue’s fork.

Keshav Patel → made their first commit to this issue’s fork.

Keshav Patel → made their first commit to this issue’s fork.

Replaced Drupal\locale\Locale::config() with \Drupal::service('locale.config_manager'), Verified the Checks Using core/scripts/dev/commit-code-check.sh --cached (All Checks Passed). Please Review

Keshav Patel → made their first commit to this issue’s fork.

MR 6520 is passed, Please Review.

Used the following reference to build: https://www.drupal.org/docs/core-modules-and-themes/core-modules/ckedito... →

Keshav Patel → made their first commit to this issue’s fork.

Renamed ColourDefinitionSniff to ColorDefinitionSniff, Please Review.

Keshav Patel → made their first commit to this issue’s fork.

Updated the method name and comment as per #29, Please review.

Keshav Patel → made their first commit to this issue’s fork.

Keshav Patel → made their first commit to this issue’s fork.

Changed it to be rendered as error message, instead of status message. Please review.

Keshav Patel → made their first commit to this issue’s fork.

Created MR as per #3.

@longwave Removed the comment from css files please review.

Changed 'const' to 'let', Please Review.