🇮🇳India @praveenpb

@mfb One more question, for existing paths which has an extra leading or trailing spaces already, this will return FALSE and consider as an invalid url. What would be the impact? Will it break some feature in existing sites?

@mfb Okay, makes sense.

One quick question, cant we sanitize the $path variable to remove space instead of handling it with the BadRequestException?

For example, $path = trim($path); resolves the first issue I guess.

praveenpb → created an issue.

praveenpb → changed the visibility of the branch 1.x to active.

@pfrenssen, The problem of using auto-generated patch is that it changes whenever someone pushed something to the MR, and it will be applied to our project when we run composer install/update without our knowledge. If someone pushes a buggy code it is not safe to use it, but if you create a patch manually, it will not change that patch.

@Webbeh, I thought to update the patch file name so uploaded a second one but I already removed the first one from display.

Just created a patch from the MR.

The MR4 fixed this issue. Thanks.

Codesniffer fixes.

1. Re-rolled against 3.1.x-dev.
2. Merged #3 and #4 to a single patch.

praveenpb → made their first commit to this issue’s fork.

praveenpb → made their first commit to this issue’s fork.

When I run it with --standard="core/phpcs.xml.dist", it doesn't throw any errors.
So my questions are,

1. Which one is the recommended standard to follow with custom code?
2. Is it fine if we follow what core follows? Otherwise we may face such issues if we implement non-compliant interface methods provided by core right?

Thanks

Hi cilefen, Thanks for your quick response.

Please find the details below. It seems "Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps" triggering here.

FOUND 3 ERRORS AFFECTING 3 LINES
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
47 | ERROR | Public method name "CustomCkeditor5::mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem" is not in lowerCamel format
| | (Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps)Files.TxtFileLineLength.TooLong)
90 | ERROR | Public method name "CustomCkeditor5::mapCKEditor4SettingsToCKEditor5Configuration" is not in lowerCamel format
| | (Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps)Files.TxtFileLineLength.TooLong)
97 | ERROR | Public method name "CustomCkeditor5::computeCKEditor5PluginSubsetConfiguration" is not in lowerCamel format
| | (Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps)

It seems this has been fixed in 9.5.4+ and 10.0.4+ versions. Can we close this issue?

The patch #10 resolved the issue.

praveenpb → made their first commit to this issue’s fork.

@prudloff, Thanks for updating the patch.
I thought to add support for footnotes which doesn't have value attribute(added in ckeditor4) so added Footnotes and FootnotesValue. But you might be right we can get rid of one of them.

Updated the MR and the patch accordingly.

Created a patch and MR for the ckeditor5 compatibility.

Created a patch and MR for the ckeditor5 compatibility.

praveenpb → made their first commit to this issue’s fork.