Automated Drupal 10 compatibility fixes

Changing pathologic.info.yml.

Adding composer.json file.

I will review this patch.

I have used the latest patch and tested using upgrade status.
Getting error after scanning using upgrade Status, I have attached the screenshot of it.

I will work on this.

Solved the last deprecate code issue.

According to https://www.drupal.org/node/2940031. file_create_url($uri) should be replaced with \Drupal::service('file_url_generator')->generateAbsoluteString($uri)

Reviewing this today!

Static analysis check

Running the latest version of mglaman/drupal-check, I see no remaining D10 deprecations. There are two "PHPDoc tag @return has invalid value" warnings, although this wouldn't cause an issue per se, so that is probably out of scope for this D10 compatibility issue. For the record, the changes to fix those notices would be:

diff --git a/src/PathologicSettingsCommon.php b/src/PathologicSettingsCommon.php
index 93efb36..4579be1 100644
--- a/src/PathologicSettingsCommon.php
+++ b/src/PathologicSettingsCommon.php
@@ -16,7 +16,7 @@ class PathologicSettingsCommon {
    *
    * @param array $defaults
    *   An array of default values for the configuration form fields.
-   * @return
+   * @return array
    *   The common form elements
    */
   public function commonSettingsForm(array $defaults) {
diff --git a/tests/src/Functional/PathologicTest.php b/tests/src/Functional/PathologicTest.php
index 590b0bc..15bf839 100644
--- a/tests/src/Functional/PathologicTest.php
+++ b/tests/src/Functional/PathologicTest.php
@@ -205,7 +205,7 @@ function _pathologic_content_url($path, $options) {
  *
  * @param $settings
  *   An array of settings for the Pathologic instance on the format.
- * @return
+ * @return string
  *   A format machine name (consisting of random characters) for the format.
  */
 function _pathologic_build_format($settings) {

Automated testing check
Running the automated tests with D10 locally, it flagged the following:

1) Drupal\Tests\pathologic\Functional\PathologicTest::testPathologic
Unable to install modules: module 'pathologic_test' is incompatible with this version of Drupal core.

The following change fixes that:

diff --git a/tests/modules/pathologic_test/pathologic_test.info.yml b/tests/modules/pathologic_test/pathologic_test.info.yml
index b216117..f3e0df4 100755
--- a/tests/modules/pathologic_test/pathologic_test.info.yml
+++ b/tests/modules/pathologic_test/pathologic_test.info.yml
@@ -1,7 +1,6 @@
 name: Pathologic Test
 description: Module which implements hook_pathologic_alter() for testing that functionality.
-core: 8.x
-core_version_requirement: ^8 || ^9
+core_version_requirement: ^9 || ^10
 package: Testing
 dependencies:
 - drupal:filter

Manual UI Testing

1. I enabled the module on D10 using the latest patched version and added a new "base path" for the site at admin/config/content/pathologic
2. I added Pathologic to the text format filters on Drupal core's "Basic HTML" text format.
3. I created a "Basic page" node with a link using the base path added in Step 1
3. I verified that the text format filter correctly rewrote the path to match the testing site domain when viewed.
4. I verified that a text-format specific setting also worked.

The attached patch includes the fixes above, and the interdiff shows the changes since the patch in #11.

This is in good shape. Maybe one more person involved to officially RTBC it? I've added the manual testing steps to the issue description.

I have reviewed and checked Patch #14 this works for drupal 9.5 and drupal 10.0. and 10.0.1
Screenshots of the compatibility on 9.5 and the Config on 10 have been attached for reference:

Patch works well. Can a D10 compatible release be created?

Everything looks good. Here's an updated version of the patch that removes a bunch of the style and white-space changes and makes it easier to review.

Thank you, Garrett, Sascha, Derek , for having time to maintain this important module, for needed to correct the path for links in multi-site environments in an editorial workflow on dev, staging, and production sites.

Hoping for a soft commit on the 8.x-1.x branch, to ease testing with Drupal 10
Testing now with git apply for the patch

Just amplifying what others have said before: thanks for this module, which has wide adoption! A soft commit on the default branch for testing would be great, and a release would be even better! (Given the limited amount of change involved for D10, I don't think there is much need to wait to do a release.)

Thanks, Mark, for the patch in #14
Testing with drupal:filter and CKEditor 5 and CKEditor 4

Correct URLs with Pathologic

Had Real physical testing round for Pathologic 1.0.x-dev with Drupal ~10 and Drupal ~9
With CKEditor 5 and CKEditor 4 drupal:filter

Attached the steps for the testing setup
Real-physical-testing-round-for-pathologic-1-0-x-dev--with-d10-and-d9.zip

Build Drupal ~10.0 sandbox test site and have Drush ~11.0

mkdir -p /var/www/html/sandboxes
composer create-project drupal/recommended-project:~10.0 /var/www/html/sandboxes/drupal10pathologic_1__test
cd /var/www/html/sandboxes/drupal10pathologic_1__test/
composer require drush/drush:~11.0;

Change minimum stability to dev for testing on the recommended Drupal 10 project template

composer config minimum-stability dev

Add CKEditor 4 for testing only

composer require drupal/ckeditor:~1.0

Add the Pathologic module

mkdir -p /var/www/html/sandboxes/drupal10pathologic_1__test/web/modules/contrib
cd /var/www/html/sandboxes/drupal10pathologic_1__test/web/modules/contrib/
git clone --branch '8.x-1.x' https://git.drupalcode.org/project/pathologic.git

Git Apply the patch.

cd /var/www/html/sandboxes/drupal10pathologic_1__test/web/modules/contrib/pathologic/
wget https://www.drupal.org/files/issues/2022-12-02/3289033-drupal-10-compatibility-14.patch
git apply 3289033-drupal-10-compatibility-14.patch

Change file/directory mod and ownership of files:

cd /var/www/html/sandboxes/drupal10pathologic_1__test/
sudo chmod 775 -R .;sudo chown www-data:$USER -R .;

Install with Drush

./vendor/drush/drush/drush site:install standard --yes --site-name="Pathologic 1.0.x-dev - Real physical testing with Drupal ~10" --account-name="webmaster" --account-pass="d" --account-mail="test@drupal.org" --db-url="mysql://root:123___@localhost/sandboxes_drupal10pathologic_1__test" -vvv ;

./vendor/drush/drush/drush pm:enable pathologic
./vendor/drush/drush/drush pm:enable ckeditor

Rebuild the cache:

sudo chmod 775 -R .;sudo chown www-data:$USER -R .;
./vendor/drush/drush/drush cache:rebuild

Open a browser and change the address to: http://localhost/sandboxes/drupal10pathologic_1__test/web/

Tested, Working well.
Ready to commit #14 and ready for a release.

The original creator of this fundamental Drupal module, Garrett Albright, added me as a maintainer, after I reached out about ongoing support. I've committed the D10-compatibility fix to the 8.x-1.x branch. Marking this issue as "Fixed."

Next step: cutting a new release.

Yep. I'm sorry, everyone, that I haven't kept up on this module for the past half decade, but I just don't work with Drupal that much anymore and don't have much interest in maintaining code projects in my free time anymore either. Thanks to Mark for picking up my slack and I'm glad to see this module is still proving useful to people.

@Garrett Albright: Indeed, thanks for this extremely useful module! Glad I was able to help it with a D8/D9 release, and give it a burst of attention at the time. No need to apologize for moving on. Grateful you contributed all your work when you did!

Welcome aboard, @mark_fullmer! I just was pulling a bunch of git repos on my machine and noticed a commit in here, which I didn't expect. ;) Glad to see why. Thanks for stepping in to help!

It's too bad this is the only change to the module, and the only thing that breaks D8 compatibility:

-    $new_parts = parse_url(file_create_url(\Drupal::config('system.file')->get('default_scheme') . '://' . $parts['path']));
+    $uri = \Drupal::config('system.file')->get('default_scheme') . '://' . $parts['path'];
+    $new_parts = parse_url(\Drupal::service('file_url_generator')->generateAbsoluteString($uri));

but oh well, life moves on. 😉 D8 is EOL, so we might as well cut a new release for D9/D10.

I wonder if we should call that 2.0.0-alpha1 and switch to semver now, instead of 8.x-1.0-alpha4 or something. 😅

Regardless, I think it'd be wise to sort out #2418369: Internal URL handling (language prefixes, base://, ...) and #3270030: 8.x-1.0-alpha3 breaks images in multi-language sites. before we really call this "stable".

Probably we should move this to a new issue about the next release...

Although I'm glad to see the tests changed to use assertEquals(), I found the original formatting a lot easier to read and make sense of. What do you think about reverting that (and using assertEquals()) before we cut a new release?

Thanks again!
-Derek

Yeah, honestly, I think we should make a new 2.0.x branch with what you committed, revert the commit to 8.x-1.x branch, and leave the possibility of a 8.x-1.0-whatever release that fixes the known bugs for people still stuck on D8, and allow us to move to semver for everything else D9+ moving forward. Sound good?

Thanks to everyone who worked on this issue doing reviews and testing! Nice to see so many folks interested in this module...

Thanks, Derek, for this! I wholeheartedly agree that this is an opportune time to switch to semver and make the D10 compatible version a new major version release, and that it would be sensible to have the first release be 2.0.0-alpha1, given the other issues you flagged.

The main implication with this change would be that sites in the process of updating to Drupal 10 would now need to manually change their Composer requirement to some variation of "drupal/pathologic: ^2", rather than automatically getting a compatible version with composer update. But that's standard practice, so we don't need to call out anything in particular.

I also agree that the syntax in the tests should be switched to multi-line parameters.

As far as reverting this commit for the 8.x-1.x branch, I suggest we create a new commit that performs that, rather than rewriting commit history, just for transparency.

I'll wait a few days for response/action from the community before taking initiative myself.

Created 2.0.x branch with the D10 port committed, reverted the commit from 8.x-1.x branch, created a 2.0.x-dev release, and pushed commit faea7caf to fix the test formatting in the 2.0.x branch. Not totally sure why d.o didn't see the follow-up commit, since it had the right issue number. Maybe it's because the 2.0.x release didn't yet exist and d.o didn't know it should care. ;)

Anyway, I think this issue is now done. Let's discuss future release plans at #3343541: Plan for new Pathologic releases.

Thanks again!
-Derek

Could you also create a release for the 2.0.x branch, that would support Drupal 10?

Could you also create a release for the 2.0.x branch, that would support Drupal 10?

A new alpha release from the 2.0.x branch is called out in #3343541: Plan for new Pathologic releases as part of the plan, and I agree that this would is valuable and time-sensitive for people looking to update to D10, and that first alpha release doesn't need to wait until the other fixes called out are resolved. I'm a co-maintainer, but it looks like don't have the permission to create releases; that said, I think you can expect a new release pretty soon!

Tested the dev version. Good to release.