wiris / html-integrations

The official JavaScript library for MathType, the leading formula editor and equation writer for the web by Wiris
https://wiris.com/solutions/integrations/html-editors/
MIT License
78 stars 53 forks source link

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

Open bendemboski opened 3 months ago

bendemboski commented 3 months ago

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.

xjiang-at-wiris commented 3 months ago

Hello @bendemboski

Thanks for reporting.

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

bendemboski commented 3 months ago

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 commented 3 months ago

@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.

xjiang-at-wiris commented 3 months ago

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 commented 3 months ago

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.