\Drupal\ckeditor\Plugin\CKEditorPlugin\Internal::getConfig() duplicates logic from \Drupal\ckeditor\CKEditorPluginManager::getEnabledButtons(). We should just call that instead, rather than maintaining the same logic in two places.

CommentFileSizeAuthor
#5 2696771-5.patch1.6 KBWim Leers
#2 2696771-2.patch1.54 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
2696771-2.patch1.54 KB
thpoul’s picture

thpoul CreditAttribution: thpoul at Pixual commented
Status: Needs review » Reviewed & tested by the community

Thank you! Patch looks great and is a nice DX change :) RTBCing

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2696771-2.patch, failed testing.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs work » Reviewed & tested by the community
FileSize
2696771-5.patch1.6 KB

Conflicted with #2696557: 404 error for CKEditor config.js file when using text format without buttons from \Drupal\ckeditor\Plugin\CKEditorPlugin\Internal. Trivial conflict to resolve.

  • catch committed 841be6b on 8.2.x 
    Issue #2696771 by Wim Leers, thpoul: Minor clean-up of \Drupal\ckeditor\...

  • catch committed b5a6df6 on 8.1.x 
    Issue #2696771 by Wim Leers, thpoul: Minor clean-up of \Drupal\ckeditor\...
catch’s picture

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

Committed/pushed to 8.2.x and cherry-picked to 8.1.x. Thanks!

Wim Leers’s picture

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

Yay, thanks!

Status: Fixed » Closed (fixed)

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