Problem/Motivation

See #3566536: [meta] eliminate core .module files

Proposed resolution

  • Deprecate ckeditor5_filter_format_edit_form_submit() and move its logic in Ckeditor5Hooks::filterFormatEditFormSubmit()
  • Deprecate _update_ckeditor5_html_filter() and move its logic in Ckeditor5Hooks::updateCkeditor5HtmlFilter()
  • Deprecate _ckeditor5_theme_css() and move its logic in Ckeditor5Hooks::themeCss()

Note that they are either underscore or form callbacks, and it was decided that will be deprecated in 11.4 and removed in 12

Remaining tasks

None.

User interface changes

None.

Introduced terminology

None.

API changes

  • New Ckeditor5Hooks public methods: ::filterFormatEditFormSubmit(), ::updateCkeditor5HtmlFilter()
  • All procedural code left in ckeditor5.module is deprecated.

Data model changes

None.

Issue fork drupal-3566792

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

claudiu.cristea created an issue. See original summary.

claudiu.cristea opened merge request !14292

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Assigned: claudiu.cristea » Unassigned
Status: Active » Needs review

Ready for review

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

Makes sense, let's do it.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

nicxvan’s picture

nicxvan commented
Status: Reviewed & tested by the community » Needs work

Needs a rebase.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs work » Reviewed & tested by the community

Moving back to RTBC, as I've only did the rebase.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

FWIW, there is one known calls to this function in the ckeditor premium module: https://git.drupalcode.org/search?group_id=2&scope=blobs&search=%22_cked... (the second is repackaging the premium module)

nicxvan’s picture

nicxvan commented

I think as a standard practice we should deprecate all functions whether there is a replacement or not.

This is initiative is touching so many modules or may be less disruptive that way.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Reviewed & tested by the community » Needs work

Agree

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs work » Needs review

Deprecated instead of deleting. Added CR https://www.drupal.org/node/3570104

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Issue summary: View changes

Updating IS

claudiu.cristea opened merge request !14606

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Title: Convert _ckeditor5_theme_css() to a method » Deprecate remaining ckeditor5.module procedural code
Issue summary: View changes

There are 3 issues that are all touching ckeditor5.module and are heavily conflicting:

I've merged all together in this ticket. I will close the others as duplicates and update the IS

claudiu.cristea closed merge request !14292

dcam’s picture

dcam commented
Status: Needs review » Needs work

The MR mostly looks good, but I found one issue and left a suggestion for it.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Needs work » Needs review

Ready for e new review

dcam’s picture

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

I gave the MR another once-over and didn't notice anything else. All feedback was addressed. This looks good to me.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Alright, let's do it. I did wonder about moving that $attach closure to an actual method, but it doesn't really matter.

Committed and pushed 90f9762bceb to main and 0aba084e23f to 11.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • longwave committed 0aba084e on 11.x 
    task: #3566792 Deprecate remaining ckeditor5.module procedural code

By...

longwave closed merge request !14606

  • longwave committed 90f9762b on main 
    task: #3566792 Deprecate remaining ckeditor5.module procedural code

By...
nicxvan’s picture

nicxvan commented
Status: Reviewed & tested by the community » Fixed

Not sure why the status didn't actually update.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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