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

Data processors should not be created with the editing view document #6381

Closed
Reinmar opened this issue Mar 5, 2020 · 2 comments · Fixed by ckeditor/ckeditor5-engine#1831

Comments

@Reinmar
Copy link
Member

Reinmar commented Mar 5, 2020

Currently we do this in editors:

this.data.processor = new HtmlDataProcessor( this.editing.view.document );

Which makes no sense. Why passing the editing document to the data processor used in the data pipepline?

The problem is that DataController does not have its own document right now. It creates its document on the fly in toView(). This will need to be changed so DataController#constructor() exposes its #viewDocument. When creating a data processor you will do this:

this.data.processor = new HtmlDataProcessor( this.data.viewDocument );

Note: Unfortunately, there will be no symmetry between the editing and data controllers. In the former, we keep only the view instance. In the latter, we won't have such an instance – we'll keep the view document directly in the controller. We discussed this with @scofalik and decided to go with this imperfect solution as we don't know yet if we'll ever need that view and what will be its shape (it's related to the performance optimization proposed by @scofalik to not recreate the entire view document on every editor.getData()).

@Reinmar Reinmar added the type:bug This issue reports a buggy (incorrect) behavior. label Mar 5, 2020
@Reinmar
Copy link
Member Author

Reinmar commented Mar 5, 2020

Note: after this change, toView() should not create its fresh document.

@Reinmar
Copy link
Member Author

Reinmar commented Mar 5, 2020

Remember to update the tutorial about creating your own editor. I think the same code was used there.

@Reinmar Reinmar added this to the iteration 30 milestone Mar 5, 2020
@oleq oleq assigned oleq and unassigned pomek Mar 12, 2020
Reinmar added a commit to ckeditor/ckeditor5-engine that referenced this issue Mar 12, 2020
Other: `DataController` will now use a single instance of the view document for all its operations (`DataController#viewDocument`). Closes ckeditor/ckeditor5#6381.
Reinmar added a commit to ckeditor/ckeditor5-core that referenced this issue Mar 12, 2020
Reinmar added a commit to ckeditor/ckeditor5-editor-balloon that referenced this issue Mar 12, 2020
Reinmar added a commit to ckeditor/ckeditor5-editor-classic that referenced this issue Mar 12, 2020
Reinmar added a commit to ckeditor/ckeditor5-editor-decoupled that referenced this issue Mar 12, 2020
Reinmar added a commit to ckeditor/ckeditor5-editor-inline that referenced this issue Mar 12, 2020
Reinmar added a commit to ckeditor/ckeditor5-ui that referenced this issue Mar 12, 2020
Reinmar added a commit to ckeditor/ckeditor5-watchdog that referenced this issue Mar 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment