Automated Drupal 10 compatibility fixes

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

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 🐛 Internal URL handling (language prefixes, base://, ...) Needs work and 🐛 8.x-1.0-alpha3 breaks images in multi-language sites. Fixed 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 🌱 Plan for new Pathologic releases Active .

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 🌱 Plan for new Pathologic releases Active 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!

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

Tested the dev version. Good to release.