dev/drupal#169 - Fix namespacing in kcfinder

[demeritcowboy]

Fixes an error when uploading images to CKEditor via KCFinder, as reported in https://civicrm.stackexchange.com/questions/41023/cannot-upload-images-using-ckeditor-on-message-template-after-5-45-upgrade

Specifically, this revises #335 to avoid namespace issues.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[tapashdatta]

This works @demeritcowboy. Thanks

[colemanw]

Unrelated test fails.

[demeritcowboy]

Thanks. I'll put up a 5.45 backport in case there's a .1 release.

[totten]

On the explanation, "fix namespacing" is a little opaque. I searched a bit and added a link to Stackexchange, which seems to be the original error report.

So to clarify scope of impact... the original issue dev/drupal#169 was about environmental-compatibility. Did this regression impact all environments or just some environments? Are we missing some coverage here (either wrt r-run or phpunit)?

[seamuslee001]

@totten it only affected Drupal 8./9 AFAIK as that is the only one where the namespacing is important.

[demeritcowboy]

Yes it should just be drupal 8/9.

I'd say the coverage gap on 335 was mostly a review fail, but with a partial excuse. I tested it on drupal 7 and tried to test it on drupal 8 but kcfinder didn't work on drupal 8 so I didn't think too much more about it. Or at least it doesn't work out of the box - obviously some people have a workaround.

And for the description here I should have linked directly to the comment on 335. And yes it's only partly related to dev/drupal169 - 335 was something @seamuslee001 put up as a followup to the fix for dev/drupal169 for completeness.

[totten]

Yeah, the disposition of ckeditor/kcfinder/cms-compat is confusing. And testing something like this is... rather persnickity...

FWIW, the report on StackExchange had some D7 indicators (sites/all/modules is more common D7), and I was perplexed by the idea that civicrm-packages/kcfinder/core/class/browser.php has different namespacing based on CMS revision. So I retested on D7 - for me, (a) the error arises on D7 and (b) the patch solves it on D7. 👍

(So in the RN's, I omitted any qualifier about CMS/environment.)

[demeritcowboy]

Interesting. I just tried again with my local drupal 7 and it's fine without the backslash. Maybe some other factor? Anyway it seems fixed for everybody now.

Trying to think if I can blame something on composer...