Problem/Motivation

Upon upgrading to 8.x-1.1, I am unable to use any instance of CKeditor due to this undefined method.

Error: Call to undefined method Drupal\\video_filter\\Plugin\\CKEditorPlugin\\VideoFilter::t() in Drupal\\video_filter\\Plugin\\CKEditorPlugin\\VideoFilter->getButtons() (line 56 of video_filter/src/Plugin/CKEditorPlugin/VideoFilter.php)

I believe this is due to the fix in #3134309: t() calls should be avoided in classes not implementing StringTranslationTrait in the class.

Steps to reproduce

Upgrade to 8.x-1.1 and attempt to edit some content that has a WYSIWYG field with CKeditor.

Proposed resolution

Implement StringTranslationTrait on VideoFilter.

Remaining tasks

Create issue branch and create commit.

User interface changes

N/A

API changes

N/A

Data model changes

N/A

CommentFileSizeAuthor
#10 Screen Shot 2022-09-22 at 11.04.47 AM.png359.72 KBdigitalfrontiersmedia
#8 video_filter-use-stringtranslationtrait-3308405-8.patch747 bytesdoublejam

Issue fork video_filter-3308405

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

pianomansam created an issue. See original summary.

pianomansam opened merge request !2

pianomansam’s picture

pianomansam commented
Status: Active » Needs review

Issue branch and Pull Request have been created. Ready for review.

adamevertsson’s picture

adamevertsson commented

I'm not a developer, but when I implement this code change, the errors in watchdog disappear and I don't get en error, but I still can't edit the node. Now it just displays a white screen.

adamevertsson’s picture

adamevertsson commented

I can also add that this bug makes it impossible to edit text formats in Drupal whereever Video Filter is activated.

baysaa’s picture

baysaa commented
Status: Needs review » Reviewed & tested by the community

Patch from MR works for me. Fixes the exception and videos can be added via the Video Filter button on the WYSIWYG.

adamevertsson’s picture

adamevertsson commented

You are right @Baysaa. A new test works correctly. I don't know what went wrong the first time. 🤷
RTBC.

doublejam’s picture

doublejam commented
StatusFileSize
new video_filter-use-stringtranslationtrait-3308405-8.patch747 bytes

Until MR gets merged here is pianomansam's MR as patch.

indigoxela’s picture

indigoxela commented

I ran into this problem in an upgrade from core 9.3 to 9.4. (together with a module update to 8.x-1.1) I can confirm that the patch / mr fixes the problem. Otherwise the upgrade completely fails.

digitalfrontiersmedia’s picture

digitalfrontiersmedia
Location Sarasota, FL
commented
StatusFileSize
new Screen Shot 2022-09-22 at 11.04.47 AM.png359.72 KB

@doublejam, no need to post a patch. The patch file is already listed with the MR:
Merge Request plain diff file link highlighted

MegaChriz made their first commit to this issue’s fork.

MegaChriz opened merge request !4

MegaChriz opened merge request !5

megachriz’s picture

megachriz
Primary language Dutch
commented

I've added some test coverage for this bug. The test is inspired by \Drupal\Tests\ckeditor\Functional\CKEditorLoadingTest. I added a tests only branch for this to demonstrate the bug. I encountered some issues in creating a merge request for it and retried it with duplicating the tests only branch for which creating a merge request also failed. Just as I'm writing this, I see the merge request for the tests only branch finally getting created.

Let's see if the tests only branch fails tests and the branch with the fix passes.

MegaChriz closed merge request !5

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Parent issue: » #3307601: Plan for Video Filter 8.x-1.2

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Version: 8.x-1.1 » 8.x-1.x-dev
Status: Reviewed & tested by the community » Fixed

Committed. Thank you.

Status: Fixed » Closed (fixed)

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