Data loss in old 3x version of module: everything before last colon in every footnote is lost when using <fn> instead of [fn] syntax

This is the root cause: 🐛 XSS attribute filtering is inconsistent and strips valid attributes Needs work . Thanks for the link, @catch!

Changing that the "data-value" attribute is used instead of "text" resolves the issue with data loss associated with colons.

The patch is only for Ckeditor 5 and does not migrate any old data to the new format.

I am coming from the old format and have thousands of old footnotes. Is there a possible fix to not lose data and having the colon fix ?

I have to use this path https://drupal.org/node/3381594 to support the legacy update.

@ayalon if you're trying to update from pre-ckeditor5 to ckeditor5, it is possible that the patch above will work. By 'old format' I think @miikamakarainen means the 'old ckeditor5 format', not 'the pre-ckeditor5 format' - however I haven't tested this yet.

Marking needs review since there's a patch here.

I did some further testing. The patch fixes the issue with the colon. But the same happens with apostroph.

CKEditor replaces an apostrophe ' with '. All data after d' is lost for good.

I think we need to extend this fix.

Added the fix for the apos as well.. tested it with different variations and seems to be working well.
Note you need to disable/enable the filter to get the new attribute in the allowed HTML tags if you have the "Limit allowed HTML tags and correct faulty HTML" filter enabled as well.
Added also the check if the value isn't set(from https://www.drupal.org/node/3381366 → )

Please ignore #9 and use this one instead. My bad.

Thanks for your work on this!

I believe we also need an update hook to convert any stored data from <fn value="" text="footnote text here"></fn> to <fn value="" data-text="footnote text here"></fn> before this change could be committed.

There is a bit of formatting/coding standards scope creep in there, but it looks trivial so that seems fine.

I believe that should do it, though I think it needs to be a batch update hook. I suppose as is if it fails a user will have to run it again and it will have less to do until it eventually passes... but not great.

I tested this locally using the example in the issue summary and the update hook converted the text attribute correctly to data-text.

I doubt an update hook is viable. There could be millions of revisions.

It’d be safer to continue to support the old format, but recommend and generate only the new format.

Fair point. Anyways working on 4x branch for eg https://www.drupal.org/project/footnotes/issues/3405986#comment-15385557 ✨ Switch to Modal window to allow more editor control Fixed and other breaking changes from the issue queue here, so perhaps a drush command instead of upgrade hook so someone can opt-in to a 'use at own risk' upgrade path and this can target 4x as well.

Then backporting to 3x branch eg in a follow-up can do as you suggest: support old and new formats for those not wanting to upgrade to 4x.

Ill revert the update hook from the merge request.and after doing so will target this to 4x so breaking change like this can be okay unless others disagree with this proposed route forward.

Okay I reverted my update hook and changed to RTBC since I've now not made any chance and have confirmed that the new functionality here works as expected with data-text. Given this is now targeting 4x branch, the breaking change is fine. I created a follow-up issue here 🐛 Backport switch from text to data-text attribute, maintaining support for text attribute Active to backport to 3x which does actually then need to avoid the breaking change and continue to support text attribute as per #14.

Okay switching back to 3.1.x-dev as target since 4.0.x branch already uses data-text/data-value and does not have this issue.

You can see the Roadmap here for 4.0.x stable 🌱 Roadmap to stable 4x branch Active

I'd like to make use of this, but the patch from #10 doesn't seem to apply anymore to the latest revision of 3.1.x-dev. Is it straightforward to reroll?

Other than some more support for auto-converting paste from Word (not a feature anyways in 3x) is there something blocking you from upgrading out of curiosity? Homepage has instructions. I'm supporting the 4x branch and none of the previous maintainers are involved any more so I don't expect you'll get any support on the 3x branch any more unfortunately.

Thanks for the update. I guess I don't understand why, if there's no development happening on the v3 branch, the code would have changed enough to make the patch invalid! I'll have to look into it myself.

We do want to upgrade to v4 when possible, but the issues that arose from updating to CKE 5 were so time-consuming that the client in question just wants to get existing footnotes fixed first before assessing the time required for an upgrade. Not a lot of hours available to put into the work. I'm sure the upgrade path you pointed out has taken a lot of possible formats into consideration, but there are thousands of entities using footnotes on the site and it's hard to trust that we won't end up writing custom queries and upgrade hooks to fix the edge cases.

We've update many sites with tens of thousands of content items without issue and hundreds of other sites have upgraded according to the module stats so maybe give it a shot. It's just a matter of running the drush command in a local site.

Sorry I can't support multiple versions but it's mostly in my free time before my daughter wakes up each morning and I'm supporting a fair number of other modules as well. Thanks for your understanding.