Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Math plugin won't work after introducing #document property to DomConverter (and other classes) #6362

Closed
pomek opened this issue Mar 2, 2020 · 2 comments
Labels
package:mathtype type:bug This issue reports a buggy (incorrect) behavior.

Comments

@pomek
Copy link
Member

pomek commented Mar 2, 2020

In this issue – #6091 – we change our API. All breaking changes are described in the PR – ckeditor/ckeditor5-engine#1826 (comment).

When I was working on the all-features sample, I wanted to add Math plugin to the editor. My all branches were checked out on changes related to #6091 and unfortunately, Math plugin crashed.

It threw an error:
image

I went through their sources and what I can notice:

  • @wiris/ckeditor5-math/src/conversion/mathmldataprocessor.js, it extends our XmlDataProcessor which changed its API. The following fix should help:

    export default class MathmlDataProcessor extends XmlDataProcessor {
    
    	/**
    	 * Creates a new instance of the MathML data processor class.
    	 *
    	 * @param {module:engine/view/document~Document} document
    	 * @param {Object} options Configuration options.
    	 * @param {Array<String>} [options.namespaces=[]] A list of namespaces allowed to use in the XML input.
    	 */
    	constructor( document, options = {} ) {
    
    		// Call XmlDataProcessor's constructor
    		super( document, options );
    
    		// Use BR_FILLER instead of NSBP_FILLER to identify <br/> and remove them when retrieving data
    		this._domConverter = new DomConverter( document, { blockFiller: BR_FILLER } );
    	}
  • @wiris/mathtype-ckeditor5/src/conversion/downcast.js - L31 creates an instance of HtmlDataProcessor. This class requires an instance of view.Document now. It could be taken from conversionApi:

    const htmlDataProcessor = new HtmlDataProcessor( conversionApi.writer.document );
  • In the same file as above, L32 (CustomMathmlDataProcessor) – the same fix.

  • @wiris/mathtype-ckeditor5/src/plugin.js - L176 - missing the view document:

    editor.data.processor = new CustomMathmlDataProcessor( editor.editing.view.document );
  • @wiris/mathtype-ckeditor5/src/integration.js – L144, as above:

    const mathmlDP = new CustomMathmlDataProcessor( writer.document );

I am not sure I found everything thought.

@pomek pomek added the type:bug This issue reports a buggy (incorrect) behavior. label Mar 2, 2020
@Reinmar Reinmar added this to the iteration 30 milestone Mar 3, 2020
@mlewand mlewand modified the milestones: iteration 30, iteration 31 Mar 13, 2020
@mlewand mlewand modified the milestones: iteration 31, iteration 30 Mar 16, 2020
@mlewand
Copy link
Contributor

mlewand commented Mar 16, 2020

Also @pomek listed changed in MathType's ckeditor5-demo {which is a branch for the upcoming rewrite) wiris/html-integrations@ckeditor5-demo...pomek:ckeditor5-demo

@mlewand
Copy link
Contributor

mlewand commented Mar 19, 2020

MathType team has recently released a beta 7.19.0-beta.0 release and it's working fine 👍

@mlewand mlewand closed this as completed Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:mathtype type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

3 participants