Problem/Motivation

Discovered at https://git.drupalcode.org/project/drupal/-/merge_requests/2310/diffs#no..., while working on #3273983: Do not assume that plugin supporting <tag attr> also supports <tag> in SourceEditingRedundantTags and upgrade path.

Steps to reproduce

N/A

Proposed resolution

Update CKEditor5PluginDefinition::getElements() to match ::getConditions(), ::getLibrary() and ::getAdminLibrary()

Remaining tasks

Do it.

User interface changes

None.

API changes

CKEditor5PluginDefinition::getElements() no longer can return false|string[], but always string[]. It may now throw a \LogicException.

Data model changes

None.

Release notes snippet

N/A

CommentFileSizeAuthor
#2 3283776-2.patch1.53 KBWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Active » Needs review
FileSize
3283776-2.patch1.53 KB
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Needs review » Reviewed & tested by the community

This would indeed make ::getElements more consistent with ::getLibrary, ::getAdminLibrary, ::getCreatableElements, and ::getConditions which I raised on the comment linked in the IS. +1 for this.

  • catch committed 24dc4ef on 10.0.x 
    Issue #3283776 by Wim Leers, lauriii: Make CKEditor5PluginDefinition::...
  • catch committed ee3835b on 10.1.x
    Issue #3283776 by Wim Leers, lauriii: Make CKEditor5PluginDefinition::...
  • catch committed 53f0644 on 9.5.x
    Issue #3283776 by Wim Leers, lauriii: Make CKEditor5PluginDefinition::...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Seems OK to do this small clean-up while ckeditor is still in beta.

Committed/pushed to 10.1.x, cherry-picked to 10.0.x and 9.5.x, thanks!

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

Many thanks!

Status: Fixed » Closed (fixed)

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

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Version: 9.5.x-dev » 9.4.x-dev
Status: Closed (fixed) » Patch (to be ported)

CKEditor 5 is experimental in 9.4.x. We should backport this change to 9.4.x.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Patch (to be ported) » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed c766b87 and pushed to 9.4.x. Thanks!

  • alexpott committed c766b87 on 9.4.x authored by catch 
    Issue #3283776 by Wim Leers, lauriii: Make CKEditor5PluginDefinition::...

Status: Fixed » Closed (fixed)

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