html-integrations icon indicating copy to clipboard operation
html-integrations copied to clipboard

[CKEditor5] Tracked insertions of equations can produce un-accept-able suggestions

Open bendemboski opened this issue 1 year ago • 5 comments

Description

For some time now, CKEditor will sometimes downcast markers as attributes on elements, for example data-suggestion-start-before or data-suggestion-end-after. So if an equation is inserted with track changes enabled, the result of an editor.getData() call will be something like:

<math
  xmlns="http://www.w3.org/1998/Math/MathML"
  data-suggestion-end-after="insertion:e9b71d7f34f79a851e753d4518b69e1c5:12345"
  data-suggestion-start-before="insertion:e9b71d7f34f79a851e753d4518b69e1c5:12345"
>
  <mo>∞</mo>
</math>

When this HTML us upcast, this code will store those marker data- attributes in the formula property on the model, and will use that property to produce HTML in subsequent downcasts, whether the suggestion should still be present or not.

This means that if you accept the suggestion, then serialize/downcast the model to an HTML string, then reload/upcast it, the suggestion will re-appear when it shouldn't.

Environment

I'm using @wiris/mathtype-ckeditor5 v7.24.4. I haven't been able to upgrade/test a more recent version, but from looking at the code I'm pretty sure it will still reproduce.

Steps to reproduce

  1. Create an editor with the MathType plugin and the CKEditor track changes plugin installed
  2. Enable track changes
  3. Insert an equation
  4. Call editor.setData(editor.getData()) (this will cause this code to create a mathml model element whose formula attribute contains an HTML string that include the data-suggestion- HTML attributes)
  5. Accept the equation insertion suggestion
  6. Call editor.setData(editor.getData())

Expected result

The equation is plan content -- not an insertion suggestion

Actual result

The equation is an insertion suggestion

Other details

It's not safe for the plugin to assume that during upcast any/all attributes on the view are part of the MathML markup because markers and potentially other editor features might downcast into attributes that get stored on the <mathml> HTML element. Potential options I can think of:

  1. The plugin could have an allow-list of actual MathML attributes to include when creating the HTML string to put in the formula model attribute
  2. The plugin could ignore all data- attributes, or specifically data-.+-(start|end)-(before|after) attributes to target these marker attributes specifically
  3. The plugin could change its upcasting/downcasting strategy so there is some wrapper element around the <mathml> element, which would ensure that other editor features like markers would apply their attributes to that wrapper element rather than the <mathml> element

I think (3) is probably the best/safest option, but (1) might be a decent solution as well. (2) seems risky as it wouldn't necessarily ensure that the plugin would play nicely with other editor features that might write out attributes other than data- attributes.

bendemboski avatar Aug 03 '24 16:08 bendemboski

Hello @bendemboski

Thanks for reporting.

We tried to reproduce, but we couldn't see the error. Could you please provide us with a demo?

xjiang-at-wiris avatar Aug 07 '24 10:08 xjiang-at-wiris

I don't know how to set up a test app that runs the MathType plugin because of the file: protocol restriction - can you point me to an example I can use as a starting point?

On Wed, Aug 7, 2024, 1:10 PM Xinyu Jiang @.***> wrote:

Hello @bendemboski https://github.com/bendemboski

Thanks for reporting.

We tried to reproduce, but we couldn't see the error. Could you please provide us with a demo?

— Reply to this email directly, view it on GitHub https://github.com/wiris/html-integrations/issues/999#issuecomment-2273112004, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEIPGPRDKQYSQWFDGXVRXDZQHXCFAVCNFSM6AAAAABL6BVI4WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZTGEYTEMBQGQ . You are receiving this because you were mentioned.Message ID: @.***>

bendemboski avatar Aug 07 '24 10:08 bendemboski

@xjiang-at-wiris can you point me to a sample repo or some documentation that will tell me how to put together a repro? I'm not sure how to get the WIRIS plugin running in a demo application.

bendemboski avatar Aug 16 '24 13:08 bendemboski

Hi @bendemboski!

Sorry for keeping you waiting. Don't worry about the demo issue, we have it under control and are working on a solution.

We'll let you know as soon as the development is complete.

Sorry for the inconvenience!

xjiang-at-wiris avatar Aug 22 '24 08:08 xjiang-at-wiris

Hi @bendemboski!

MathType is currently not designed to be compatible with the TrackChanges plugin. In fact, as long as TrackChanges mode is turned on, MathType's buttons will be disabled. We do plan to make MathType compatible with TrackChanges in the long term, but it is not a priority at this stage.

Our temporary solution is to clear the data-suggestion-x attribute inside the <mathml> when the user accepts or rejects a suggestion. We will also disable the ability to modify formulas via double-click when in TrackChanges mode to prevent errors.

We hope this plan can solve your problem. We will notify you once the development is completed.

Thank you for your suggestion.

xjiang-at-wiris avatar Aug 29 '24 10:08 xjiang-at-wiris

Hi @bendemboski,

We'll proceed and close this issue.

If you need further assistance, please feel free to open a new issue or contact us at [email protected].

carla-at-wiris avatar Sep 12 '25 12:09 carla-at-wiris

@carla-at-wiris what is the resolution? It's closed as completed -- was there a code change to address it?

bendemboski avatar Sep 12 '25 16:09 bendemboski