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

Custom elements transform into plaintext if cursor at end of element and paragraph #7358

Closed
Discordius opened this issue Jun 2, 2020 · 23 comments
Assignees
Labels
resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). squad:core Issue to be handled by the Core team. support:1 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@Discordius
Copy link

Discordius commented Jun 2, 2020

📝 Provide detailed reproduction steps (if any)

I am setting up a custom MathJax integration based on https://github.com/isaul32/ckeditor5-math.

Somehow this happens sometimes when I have an inline-MathJax block at the end of a paragraph and press any input key:

As you can see, somehow even though I have no text selected it just completely deletes everything between the first mathtex element in the paragraph and the end of the paragraph.

There are no errors in the console. Here are my converters and setup code:

Schema:

defineSchema() {
    const schema = this.editor.model.schema;

    schema.register( 'mathtex', {
        allowWhere: '$text',
        isInline: true,
        isObject: true,
        allowAttributes: [ 'equation', 'type' ]
    } );

}

Converters

_defineConverters() {
    const conversion = this.editor.conversion;
    const mathConfig = Object.assign( defaultConfig, this.editor.config.get( 'math' ) );

// Model -> View (element)
conversion.for( 'editingDowncast' )
    .elementToElement( {
        model: 'mathtex',
        view: ( modelItem, viewWriter ) => {
            const widgetElement = createMathtexEditingView( modelItem, viewWriter, false );
            return toWidget( widgetElement, viewWriter );
        }
    }, { priority: 'high' } )


// Model -> Data
conversion.for( 'dataDowncast' )
    .elementToElement( {
        model: 'mathtex',
        view: ( modelItem, viewWriter ) => createMathtexView( modelItem, viewWriter, false )
    } )

// Create view for editor
function createMathtexEditingView( modelItem, viewWriter, display ) {
    const equation = modelItem.getAttribute( 'equation' );

    const styles = 'user-select: none; ' + ( display ? '' : 'display: inline-block;' );
    const classes = 'ck-math-tex ' + ( display ? 'ck-math-tex-display' : 'ck-math-tex-inline' );

    // CKEngine render multiple times if using span instead of div
    const mathtexView = viewWriter.createContainerElement( 'span', {
        style: styles,
        class: classes
    } );

    // Div is formatted as parent container
    const uiElement = viewWriter.createUIElement( 'div', { class: 'ck-math-tex-ui' }, function( domDocument ) {
        const domElement = this.toDomElement( domDocument );
        renderEquation( equation, domElement, display, false );
        return domElement;
    } );

    // const innerText = viewWriter.createText( '{' + equation + '}' );
    viewWriter.insert( viewWriter.createPositionAt( mathtexView, 0 ), uiElement );

    return mathtexView;
}

For more details, you can also look at the code in this file.

I have no idea why this is happening, and don't really have any pointers to how to debug this. It very likely has something to do with the createUIElement section, since when I replace the createUIElement( 'div', ... with createUIElement( 'span', ...) I no longer get the error you see in the gif above, and instead get the following error:

Which isn't really much better.

It also doesn't really matter what I put inside of the UIElement, or whether I have a call to MathJax in it. The bug still happens with the following UIElement renderer:

const uiElement = viewWriter.createUIElement( 'span', { class: 'ck-math-tex-ui' }, function( domDocument ) {
    const domElement = this.toDomElement( domDocument );
    domElement.innerHTML = '<b>this is ui element</b>';
    return domElement;
} );

viewWriter.insert( viewWriter.createPositionAt( mathtexView, 0 ), uiElement );

Which then produces the following bug:

I can even fully remove the call to createUIElement, and it still happens (though now the container element is empty, so it's hard to see unless you observe the model in the debugger).

📃 Other details

  • Browser: Chrome 83
  • OS: MacOS
  • CKEditor version: 19
  • Installed CKEditor plugins: custom plugin, plus default Balloon Block plugins

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@Discordius Discordius added the type:bug This issue reports a buggy (incorrect) behavior. label Jun 2, 2020
@Discordius Discordius changed the title Custom elements transform into plaintext if input at end of element and paragraph Custom elements transform into plaintext if cursor at end of element and paragraph Jun 2, 2020
@Mgsy
Copy link
Member

Mgsy commented Jun 15, 2020

Hi! Could you please create some simplified code snippets with a minimal amount of code necessary to reproduce this issue? TBH, you attached a lot of code with some custom functions and we won't be able to analyze such large amount of lines. 

Also, I see that you use UI elements. Note that those elements should be used only for displaying some additional elements in a parent container. Those can't be converted to the model.

@Mgsy Mgsy added the pending:feedback This issue is blocked by necessary feedback. label Jun 15, 2020
@Discordius
Copy link
Author

Discordius commented Sep 24, 2020

Ok, I finally got around to this. @Mgsy

Here is a minimal reproduction: https://github.com/Discordius/ckeditor-bug-repro

Here is a gif showing the error:

Sep-23-2020 20-32-21

The reproduction is really simple.

  1. Move your cursor to the end of the first line
  2. Press Command/CTRL+i
  3. Type any character

@Discordius
Copy link
Author

Here is literally all the code necessary to generate this bug as a plugin:

import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import Widget from '@ckeditor/ckeditor5-widget/src/widget';

export default class MathEditing extends Plugin {
	static get pluginName() {
		return 'Math';
	}

	static get requires() {
		return [ Widget ];
	}

	static get pluginName() {
		return 'MathEditing';
	}

	init() {
		this._defineSchema();
		this._defineConverters();
	}

	_defineSchema() {
		const schema = this.editor.model.schema;

		schema.register( 'mathtex', {
			allowWhere: '$text',
			isInline: true,
			isObject: true,
		} );
	}

	_defineConverters() {
		const conversion = this.editor.conversion;

		// View -> Model
		conversion.for( 'upcast' )

			.elementToElement( {
				view: {
					name: 'span',
					classes: [ 'math-tex' ]
				},
				model: ( viewElement, { writer } ) => {
					return writer.createElement( 'mathtex');
				}
			} )

		// Model -> View (element)
		conversion.for( 'editingDowncast' )
			.elementToElement( {
				model: 'mathtex',
				view: ( modelItem, { writer } ) => writer.createRawElement( 'div', null, function( domElement ) {
					domElement.innerText = '[equation]'
					return domElement;
				} )
			} )

		// Model -> Data
		conversion.for( 'dataDowncast' )
			.elementToElement( {
				model: 'mathtex',
				view: ( modelItem, { writer } ) => writer.createContainerElement( 'span', {
					class: 'math-tex'
				} )
			} )
	}
}
import BalloonEditorBase from '@ckeditor/ckeditor5-editor-balloon/src/ballooneditor';
import Essentials from '@ckeditor/ckeditor5-essentials/src/essentials';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import Math from './example-plugin/mathediting';
import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic';

export default class BalloonEditor extends BalloonEditorBase {}

// Plugins to include in the build.
BalloonEditor.builtinPlugins = [
	Essentials,
	Paragraph,
	Math,
	Italic
];
<!DOCTYPE html>
<html lang="en">
<head>
	<meta charset="utf-8">
	<title>CKEditor 5 – balloon editor build – development sample</title>
	<style>
		body {
			max-width: 800px;
			margin: 20px auto;
		}
	</style>
</head>
<body>

<h1>CKEditor 5 – balloon editor build – development sample</h1>

<div id="editor">
	<p><strong>To see that:</strong>&nbsp;<span class="math-tex">\(\bowtie\)</span>&nbsp;is <strong>strictly</strong> weaker <i>asdasdsa&nbsp;</i></p>
</div>

<script src="../build/ckeditor.js"></script>
<script>
	BalloonEditor.create( document.querySelector( '#editor' ) )
		.then( editor => {
			window.editor = editor;
		} )
		.catch( error => {
			console.error( 'There was a problem initializing the editor.', error );
		} );
</script>

</body>
</html>

@Discordius
Copy link
Author

Discordius commented Oct 6, 2020

@Mgsy: Really sorry to ping you here, but this is a pretty high priority bug for us that is currently blocking deploying CKEditor to a broader audience on our site, and I figured this might have slipped through the cracks.

@Mgsy
Copy link
Member

Mgsy commented Oct 7, 2020

Hi, @Discordius. As I can see, your minimal implementation misses a part for handling widgets, so your math-tex element shows up as a plain text because the conversion is incorrect. I assume that you'd like to wrap it in an inline widget, so I suggest checking our Creating an inline widget guide to learn more. 

Additionally, we offer a MathType feature for handling math formulas, so maybe it will be a good choice for your use case?

@Discordius
Copy link
Author

@Mgsy Since this is a minimal reproduction, I removed all unnecessary elements. This includes the the need for it to be a widget. It's totally fine for it to be plaintext. The same bug reproduces if you make it a widget. Here is the code to reproduce the bug with the element being properly processed as a widget:

/* eslint-disable no-tabs */
import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import { toWidget } from '@ckeditor/ckeditor5-widget/src/utils';
import Widget from '@ckeditor/ckeditor5-widget/src/widget';

export default class MathEditing extends Plugin {
	static get requires() {
		return [ Widget ];
	}

	static get pluginName() {
		return 'MathEditing';
	}

	init() {
		this._defineSchema();
		this._defineConverters();
	}

	_defineSchema() {
		const schema = this.editor.model.schema;

		schema.register( 'mathtex', {
			allowWhere: '$text',
			isInline: true,
			isObject: true
		} );
	}

	_defineConverters() {
		const conversion = this.editor.conversion;

		// View -> Model
		conversion.for( 'upcast' )
			.elementToElement( {
				view: {
					name: 'span',
					classes: [ 'math-tex' ]
				},
				model: ( viewElement, { writer } ) => {
					return writer.createElement( 'mathtex' );
				}
			} )

		// Model -> View (element)
		conversion.for( 'editingDowncast' )
			.elementToElement( {
				model: 'mathtex',
				view: ( modelItem, { writer } ) => {
					const widgetElement = createMathtexEditingView( modelItem, writer, false );
					return toWidget( widgetElement, writer );
				}
			} )

		// Model -> Data
		conversion.for( 'dataDowncast' )
			.elementToElement( {
				model: 'mathtex',
				view: ( modelItem, { writer } ) => writer.createContainerElement( 'span', {class: "math-tex"} )
			} )

		// Create view for editor
		function createMathtexEditingView( modelItem, writer, display ) {

			// CKEngine render multiple times if using span instead of div
			const mathtexView = writer.createContainerElement( 'span' );

			// Div is formatted as parent container
			const uiElement = writer.createRawElement( 'div', {style: "display: inline"}, function( domElement ) {
				domElement.innerHTML = "[equation]";
				return domElement;
			} );

			writer.insert( writer.createPositionAt( mathtexView, 0 ), uiElement );

			return mathtexView;
		}
	}
}

Index.html

<!DOCTYPE html>
<html lang="en">
<head>
	<meta charset="utf-8">
	<title>CKEditor 5 – balloon editor build – development sample</title>
	<style>
		body {
			max-width: 800px;
			margin: 20px auto;
		}
	</style>
</head>
<body>

<h1>CKEditor 5 – balloon editor build – development sample</h1>

<div id="editor">
	<p>Definition: Given Cartesian frames&nbsp;<span class="math-tex">&nbsp;</span></p>
</div>
<a onClick="console.log(window.editor.getData())"> Log Contents </a>
<script src="../build/ckeditor.js"></script>
<script src="https://cdn.jsdelivr.net/npm/@ckeditor/ckeditor5-inspector@1.5.0/build/inspector.js"></script>
<script>
	BalloonEditor.create( document.querySelector( '#editor' ) )
		.then( editor => {
			window.editor = editor;
			CKEditorInspector.attach(editor);
			return editor
		} )
		.catch( error => {
			console.error( 'There was a problem initializing the editor.', error );
		} );
</script>

</body>
</html>

I've also updated the reproduction repository: https://github.com/Discordius/ckeditor-bug-repro

Here is a gif of the relevant behavior again:
Oct-07-2020 14-25-09

Alas, MathType is a software that appears to be at least as buggy as what I am working with here, and also doesn't support WYSIWYG LaTeX editing at least as far as I can tell, so it's not a good fit for what I am trying to do. I am also broadly hesitant to integrate too many closed-source components into our codebase, since bugs are frequent, and in a closed-source environment I often have no way out to fix them.

@jodator
Copy link
Contributor

jodator commented Oct 8, 2020

@Discordius - I've tested your example and it looks like some kind of quirk when using <div> here. I'm not entirely sure why this happens though. If I change div to anything else (tested with <span> and <mathtex>) it starts to work:

ps.: I've added:

this.editor.editing.mapper.on(
	'viewToModelPosition',
	viewToModelPositionOutsideModelElement( this.editor.model, viewElement => viewElement.name == 'mathtex' )
);

as suggested in the inline widget docs.

@Discordius
Copy link
Author

Discordius commented Oct 8, 2020

Yeah, but if you change it to span, different issues appear, as you can see in this screencap I pasted above:

83486484-1b4af600-a45e-11ea-822f-3ff2796019db

This error is sadly even worse and occurs even more frequently, so changing it to span doesn't really help me much. My guess is the two errors must be related, since they occur in such similar circumstances.

@jodator
Copy link
Contributor

jodator commented Oct 9, 2020

At this point, I'm not sure what's happening and I'm starting to suspect that MathJax is somehow messing with the editor content.

I've checked the linked plugin and there's something that looks like a workaround for this problem. It looks that they have that working by displaying the MathJax equation over the editor content. So the MathJax dom element is outside editor editing view DOM.

I did a quick check what renders MathJax.

  1. HTML output - was kinda unusable for my POC (lack of styles)
  2. SVG output - kinda works:

I had to hide <mjx-assistive-mml> using display:none.

However, I did not use the MathJax library but I've copied the rendered DOM as an HTML string from MathJax SVG demo.

I did an only asynchronous update of the dom node inside the editing area:

const uiElement = writer.createRawElement( 'span', { style: 'display: inline' }, function( domElement ) {
	domElement.innerHTML = '[equation]';
	setTimeout( renderSomething( domElement ), 50 );
	return domElement;
} );

render function looks like this:

function renderSomething( domElement ) {
	return () => {
		domElement.innerHTML = '<mjx-container class="MathJax CtxtMenu_Attached_0" jax="SVG" display="true" role="presentation" style="position: relative;" tabindex="0" ctxtmenu_counter="12"><svg style="vertical-align: -1.948ex;" xmlns="http://www.w3.org/2000/svg" width="22.544ex" height="5.251ex" role="img" focusable="false" viewBox="0 -1460 9964.3 2321" xmlns:xlink="http://www.w3.org/1999/xlink" aria-hidden="true"><defs><path id="MJX-2-TEX-I-1D453" d="M118 -162Q120 -162 124 -164T135 -167T147 -168Q160 -168 171 -155T187 -126Q197 -99 221 27T267 267T289 382V385H242Q195 385 192 387Q188 390 188 397L195 425Q197 430 203 430T250 431Q298 431 298 432Q298 434 307 482T319 540Q356 705 465 705Q502 703 526 683T550 630Q550 594 529 578T487 561Q443 561 443 603Q443 622 454 636T478 657L487 662Q471 668 457 668Q445 668 434 658T419 630Q412 601 403 552T387 469T380 433Q380 431 435 431Q480 431 487 430T498 424Q499 420 496 407T491 391Q489 386 482 386T428 385H372L349 263Q301 15 282 -47Q255 -132 212 -173Q175 -205 139 -205Q107 -205 81 -186T55 -132Q55 -95 76 -78T118 -61Q162 -61 162 -103Q162 -122 151 -136T127 -157L118 -162Z"></path><path id="MJX-2-TEX-N-28" d="M94 250Q94 319 104 381T127 488T164 576T202 643T244 695T277 729T302 750H315H319Q333 750 333 741Q333 738 316 720T275 667T226 581T184 443T167 250T184 58T225 -81T274 -167T316 -220T333 -241Q333 -250 318 -250H315H302L274 -226Q180 -141 137 -14T94 250Z"></path><path id="MJX-2-TEX-I-1D44E" d="M33 157Q33 258 109 349T280 441Q331 441 370 392Q386 422 416 422Q429 422 439 414T449 394Q449 381 412 234T374 68Q374 43 381 35T402 26Q411 27 422 35Q443 55 463 131Q469 151 473 152Q475 153 483 153H487Q506 153 506 144Q506 138 501 117T481 63T449 13Q436 0 417 -8Q409 -10 393 -10Q359 -10 336 5T306 36L300 51Q299 52 296 50Q294 48 292 46Q233 -10 172 -10Q117 -10 75 30T33 157ZM351 328Q351 334 346 350T323 385T277 405Q242 405 210 374T160 293Q131 214 119 129Q119 126 119 118T118 106Q118 61 136 44T179 26Q217 26 254 59T298 110Q300 114 325 217T351 328Z"></path><path id="MJX-2-TEX-N-29" d="M60 749L64 750Q69 750 74 750H86L114 726Q208 641 251 514T294 250Q294 182 284 119T261 12T224 -76T186 -143T145 -194T113 -227T90 -246Q87 -249 86 -250H74Q66 -250 63 -250T58 -247T55 -238Q56 -237 66 -225Q221 -64 221 250T66 725Q56 737 55 738Q55 746 60 749Z"></path><path id="MJX-2-TEX-N-3D" d="M56 347Q56 360 70 367H707Q722 359 722 347Q722 336 708 328L390 327H72Q56 332 56 347ZM56 153Q56 168 72 173H708Q722 163 722 153Q722 140 707 133H70Q56 140 56 153Z"></path><path id="MJX-2-TEX-N-31" d="M213 578L200 573Q186 568 160 563T102 556H83V602H102Q149 604 189 617T245 641T273 663Q275 666 285 666Q294 666 302 660V361L303 61Q310 54 315 52T339 48T401 46H427V0H416Q395 3 257 3Q121 3 100 0H88V46H114Q136 46 152 46T177 47T193 50T201 52T207 57T213 61V578Z"></path><path id="MJX-2-TEX-N-32" d="M109 429Q82 429 66 447T50 491Q50 562 103 614T235 666Q326 666 387 610T449 465Q449 422 429 383T381 315T301 241Q265 210 201 149L142 93L218 92Q375 92 385 97Q392 99 409 186V189H449V186Q448 183 436 95T421 3V0H50V19V31Q50 38 56 46T86 81Q115 113 136 137Q145 147 170 174T204 211T233 244T261 278T284 308T305 340T320 369T333 401T340 431T343 464Q343 527 309 573T212 619Q179 619 154 602T119 569T109 550Q109 549 114 549Q132 549 151 535T170 489Q170 464 154 447T109 429Z"></path><path id="MJX-2-TEX-I-1D70B" d="M132 -11Q98 -11 98 22V33L111 61Q186 219 220 334L228 358H196Q158 358 142 355T103 336Q92 329 81 318T62 297T53 285Q51 284 38 284Q19 284 19 294Q19 300 38 329T93 391T164 429Q171 431 389 431Q549 431 553 430Q573 423 573 402Q573 371 541 360Q535 358 472 358H408L405 341Q393 269 393 222Q393 170 402 129T421 65T431 37Q431 20 417 5T381 -10Q370 -10 363 -7T347 17T331 77Q330 86 330 121Q330 170 339 226T357 318T367 358H269L268 354Q268 351 249 275T206 114T175 17Q164 -11 132 -11Z"></path><path id="MJX-2-TEX-I-1D456" d="M184 600Q184 624 203 642T247 661Q265 661 277 649T290 619Q290 596 270 577T226 557Q211 557 198 567T184 600ZM21 287Q21 295 30 318T54 369T98 420T158 442Q197 442 223 419T250 357Q250 340 236 301T196 196T154 83Q149 61 149 51Q149 26 166 26Q175 26 185 29T208 43T235 78T260 137Q263 149 265 151T282 153Q302 153 302 143Q302 135 293 112T268 61T223 11T161 -11Q129 -11 102 10T74 74Q74 91 79 106T122 220Q160 321 166 341T173 380Q173 404 156 404H154Q124 404 99 371T61 287Q60 286 59 284T58 281T56 279T53 278T49 278T41 278H27Q21 284 21 287Z"></path><path id="MJX-2-TEX-LO-222E" d="M114 -798Q132 -824 165 -824H167Q195 -824 223 -764T275 -600T320 -391T362 -164Q365 -143 367 -133Q382 -52 390 2Q314 40 276 99Q230 167 230 249Q230 363 305 436T484 519H494L503 563Q587 939 632 1087T727 1298Q774 1360 828 1360Q884 1360 912 1325T944 1245Q944 1220 932 1205T909 1186T887 1183Q866 1183 849 1198T832 1239Q832 1287 885 1296L882 1300Q879 1303 874 1307T866 1313Q851 1323 833 1323Q766 1323 688 929Q662 811 610 496Q770 416 770 249Q770 147 701 68T516 -21H506L497 -65Q407 -464 357 -623T237 -837Q203 -862 165 -862Q125 -862 92 -831T55 -746Q55 -711 74 -698T112 -685Q133 -685 150 -700T167 -741Q167 -789 114 -798ZM480 478Q460 478 435 470T380 444T327 401T287 335T271 249Q271 124 375 56L397 43L431 223L485 478H480ZM519 20Q545 20 578 33T647 72T706 144T730 249Q730 383 603 455Q603 454 597 421T582 343T569 276Q516 22 515 20H519Z"></path><path id="MJX-2-TEX-I-1D467" d="M347 338Q337 338 294 349T231 360Q211 360 197 356T174 346T162 335T155 324L153 320Q150 317 138 317Q117 317 117 325Q117 330 120 339Q133 378 163 406T229 440Q241 442 246 442Q271 442 291 425T329 392T367 375Q389 375 411 408T434 441Q435 442 449 442H462Q468 436 468 434Q468 430 463 420T449 399T432 377T418 358L411 349Q368 298 275 214T160 106L148 94L163 93Q185 93 227 82T290 71Q328 71 360 90T402 140Q406 149 409 151T424 153Q443 153 443 143Q443 138 442 134Q425 72 376 31T278 -11Q252 -11 232 6T193 40T155 57Q111 57 76 -3Q70 -11 59 -11H54H41Q35 -5 35 -2Q35 13 93 84Q132 129 225 214T340 322Q352 338 347 338Z"></path><path id="MJX-2-TEX-N-2212" d="M84 237T84 250T98 270H679Q694 262 694 250T679 230H98Q84 237 84 250Z"></path><path id="MJX-2-TEX-I-1D451" d="M366 683Q367 683 438 688T511 694Q523 694 523 686Q523 679 450 384T375 83T374 68Q374 26 402 26Q411 27 422 35Q443 55 463 131Q469 151 473 152Q475 153 483 153H487H491Q506 153 506 145Q506 140 503 129Q490 79 473 48T445 8T417 -8Q409 -10 393 -10Q359 -10 336 5T306 36L300 51Q299 52 296 50Q294 48 292 46Q233 -10 172 -10Q117 -10 75 30T33 157Q33 205 53 255T101 341Q148 398 195 420T280 442Q336 442 364 400Q369 394 369 396Q370 400 396 505T424 616Q424 629 417 632T378 637H357Q351 643 351 645T353 664Q358 683 366 683ZM352 326Q329 405 277 405Q242 405 210 374T160 293Q131 214 119 129Q119 126 119 118T118 106Q118 61 136 44T179 26Q233 26 290 98L298 109L352 326Z"></path></defs><g stroke="currentColor" fill="currentColor" stroke-width="0" transform="matrix(1 0 0 -1 0 0)"><g data-mml-node="math"><g data-mml-node="mi"><use xlink:href="#MJX-2-TEX-I-1D453"></use></g><g data-mml-node="mo" transform="translate(550, 0)"><use xlink:href="#MJX-2-TEX-N-28"></use></g><g data-mml-node="mi" transform="translate(939, 0)"><use xlink:href="#MJX-2-TEX-I-1D44E"></use></g><g data-mml-node="mo" transform="translate(1468, 0)"><use xlink:href="#MJX-2-TEX-N-29"></use></g><g data-mml-node="mo" transform="translate(2134.8, 0)"><use xlink:href="#MJX-2-TEX-N-3D"></use></g><g data-mml-node="mfrac" transform="translate(3190.6, 0)"><g data-mml-node="mn" transform="translate(677.5, 676)"><use xlink:href="#MJX-2-TEX-N-31"></use></g><g data-mml-node="mrow" transform="translate(220, -686)"><g data-mml-node="mn"><use xlink:href="#MJX-2-TEX-N-32"></use></g><g data-mml-node="mi" transform="translate(500, 0)"><use xlink:href="#MJX-2-TEX-I-1D70B"></use></g><g data-mml-node="mi" transform="translate(1070, 0)"><use xlink:href="#MJX-2-TEX-I-1D456"></use></g></g><rect width="1615" height="60" x="120" y="220"></rect></g><g data-mml-node="mo" transform="translate(5212.2, 0)"><use xlink:href="#MJX-2-TEX-LO-222E"></use></g><g data-mml-node="mfrac" transform="translate(6322.9, 0)"><g data-mml-node="mrow" transform="translate(431.7, 710)"><g data-mml-node="mi"><use xlink:href="#MJX-2-TEX-I-1D453"></use></g><g data-mml-node="mo" transform="translate(550, 0)"><use xlink:href="#MJX-2-TEX-N-28"></use></g><g data-mml-node="mi" transform="translate(939, 0)"><use xlink:href="#MJX-2-TEX-I-1D467"></use></g><g data-mml-node="mo" transform="translate(1404, 0)"><use xlink:href="#MJX-2-TEX-N-29"></use></g></g><g data-mml-node="mrow" transform="translate(220, -686)"><g data-mml-node="mi"><use xlink:href="#MJX-2-TEX-I-1D467"></use></g><g data-mml-node="mo" transform="translate(687.2, 0)"><use xlink:href="#MJX-2-TEX-N-2212"></use></g><g data-mml-node="mi" transform="translate(1687.4, 0)"><use xlink:href="#MJX-2-TEX-I-1D44E"></use></g></g><rect width="2416.4" height="60" x="120" y="220"></rect></g><g data-mml-node="mi" transform="translate(8979.3, 0)"><use xlink:href="#MJX-2-TEX-I-1D451"></use></g><g data-mml-node="mi" transform="translate(9499.3, 0)"><use xlink:href="#MJX-2-TEX-I-1D467"></use></g></g></g></svg><mjx-assistive-mml role="presentation" unselectable="on" display="block"><math xmlns="http://www.w3.org/1998/Math/MathML" display="block"><mi>f</mi><mo stretchy="false">(</mo><mi>a</mi><mo stretchy="false">)</mo><mo>=</mo><mfrac><mn>1</mn><mrow><mn>2</mn><mi>π</mi><mi>i</mi></mrow></mfrac><mo data-mjx-texclass="OP">∮</mo><mfrac><mrow><mi>f</mi><mo stretchy="false">(</mo><mi>z</mi><mo stretchy="false">)</mo></mrow><mrow><mi>z</mi><mo>−</mo><mi>a</mi></mrow></mfrac><mi>d</mi><mi>z</mi></math></mjx-assistive-mml></mjx-container>';
	};
}

Maybe you could check if this approach could help you:

  1. Render the MathJax equation outside editor DOM, maybe inside some hidden container.
  2. Copy the rendered output to the editor asynchronously and do not allow MathJax to run on it again.
  3. If the HTML output messes the editor you could try SVG output.

It will be hard for us to help you more with this as it looks like we can get this working without the external library messing with the editor DOM. We are aware of some problems with the widget system and using dynamic content inside them (ie #4600 ). Additionally, we do not have, for now at least, an official plugin that uses inline widgets. This means that inline widgets have minimal support for the basic use-cases.

I hope that you could get around this 🤞

@Discordius
Copy link
Author

Sorry, but my reproduction above shows you that it doesn't rely at all on MathJax content!

It also doesn't really matter what I put inside of the UIElement, or whether I have a call to MathJax in it. The bug still happens with the following UIElement renderer:

const uiElement = viewWriter.createUIElement( 'span', { class: 'ck-math-tex-ui' }, function( domDocument ) {
const domElement = this.toDomElement( domDocument );
domElement.innerHTML = 'this is ui element';
return domElement;
} );

viewWriter.insert( viewWriter.createPositionAt( mathtexView, 0 ), uiElement );
Which then produces the following bug:

83487604-2a32a800-a460-11ea-860f-13fab519b45b

This bug reproduces without any reference to the Mathjax library, so of course it can't be caused by that.

@Discordius
Copy link
Author

Discordius commented Oct 9, 2020

I just pushed a reproduction to the repository that reproduces it with span elements without any reference to MathJax. While setting up that reproduction, I also found out one interesting pointer that might help us find the right solution here.

Here is the content from which we initialize the editor:

<div id="editor">
	<p>Definition: Given Cartesian frames&nbsp;<span class="math-tex"></span></p>
	<p>Definition: Given Cartesian frames&nbsp;<span class="math-tex"></span></p>
	<p>Definition: Given Cartesian frames&nbsp;<span class="math-tex">&nbsp;</span></p>
</div>

You can see that the first two math-tex span elements are empty, but the third one has a non-breaking space in it. And the bug only reproduces with the two that have a nothing inside of them. Here is a gif again of that happening:

Oct-09-2020 10-45-29

Sadly this doesn't really help me with fixing the bug when I edit the document, since that never routes through the data version of the document.

Just for reference and ease of reproducibility, here is again the whole code to reproduce this issue with inline elements, and no reference to LaTeX:

Index.html

<!DOCTYPE html>
<html lang="en">
<head>
	<meta charset="utf-8">
	<title>CKEditor 5 – balloon editor build – development sample</title>
	<style>
		body {
			max-width: 800px;
			margin: 20px auto;
		}
	</style>
</head>
<body>

<h1>CKEditor 5 – balloon editor build – development sample</h1>

<div id="editor">
	<p>Definition: Given Cartesian frames&nbsp;<span class="math-tex"></span></p>
	<p>Definition: Given Cartesian frames&nbsp;<span class="math-tex"></span></p>
	<p>Definition: Given Cartesian frames&nbsp;<span class="math-tex">&nbsp;</span></p>
</div>
<a onClick="console.log(window.editor.getData())"> Log Contents </a>
<script src="../build/ckeditor.js"></script>
<script src="https://cdn.jsdelivr.net/npm/@ckeditor/ckeditor5-inspector@1.5.0/build/inspector.js"></script>
<script>
	BalloonEditor.create( document.querySelector( '#editor' ) )
		.then( editor => {
			window.editor = editor;
			CKEditorInspector.attach(editor);
			return editor
		} )
		.catch( error => {
			console.error( 'There was a problem initializing the editor.', error );
		} );
</script>

</body>
</html>

mathediting.js

/* eslint-disable no-tabs */
import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import { toWidget } from '@ckeditor/ckeditor5-widget/src/utils';
import Widget from '@ckeditor/ckeditor5-widget/src/widget';

export default class MathEditing extends Plugin {
	static get requires() {
		return [ Widget ];
	}

	static get pluginName() {
		return 'MathEditing';
	}

	init() {
		this._defineSchema();
		this._defineConverters();
	}

	_defineSchema() {
		const schema = this.editor.model.schema;

		schema.register( 'mathtex', {
			allowWhere: '$text',
			isInline: true,
			isObject: true
		} );
	}

	_defineConverters() {
		const conversion = this.editor.conversion;

		// View -> Model
		conversion.for( 'upcast' )
			.elementToElement( {
				view: {
					name: 'span',
					classes: [ 'math-tex' ]
				},
				model: ( viewElement, { writer } ) => {
					return writer.createElement( 'mathtex' );
				}
			} )

		// Model -> View (element)
		conversion.for( 'editingDowncast' )
			.elementToElement( {
				model: 'mathtex',
				view: ( modelItem, { writer } ) => {
					const widgetElement = createMathtexEditingView( modelItem, writer, false );
					return toWidget( widgetElement, writer );
				}
			} )

		// Model -> Data
		conversion.for( 'dataDowncast' )
			.elementToElement( {
				model: 'mathtex',
				view: ( modelItem, { writer } ) => writer.createContainerElement( 'span', {class: "math-tex"} )
			} )

		// Create view for editor
		function createMathtexEditingView( modelItem, writer, display ) {

			// CKEngine render multiple times if using span instead of div
			const mathtexView = writer.createContainerElement( 'span' );

			// Div is formatted as parent container
			const uiElement = writer.createRawElement( 'span', {style: "font-weight: bold"}, function( domElement ) {
				domElement.innerHTML = "[equation]";
				// renderEquation( equation, domElement, mathConfig.engine, display, false );
				return domElement;
			} );

			writer.insert( writer.createPositionAt( mathtexView, 0 ), uiElement );

			return mathtexView;
		}
	}
}

ckeditor.js

/**
 * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved.
 * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
 */

// The editor creator to use.
import BalloonBlockEditorBase from '@ckeditor/ckeditor5-editor-balloon/src/ballooneditor';
import Essentials from '@ckeditor/ckeditor5-essentials/src/essentials';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import Math from './example-plugin/mathediting';
import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic';

export default class BalloonEditor extends BalloonBlockEditorBase {}

// Plugins to include in the build.
BalloonEditor.builtinPlugins = [
	Essentials,
	Paragraph,
	Math,
	Italic
];

// Editor configuration.
BalloonEditor.defaultConfig = {
	// This value must be kept in sync with the language defined in webpack.config.js.
	language: 'en'
};

@Discordius
Copy link
Author

Discordius commented Oct 17, 2020

Hey, I am really sorry to ping you again, but like, is there any way I can get any more help fixing this? This seems like a bug that surely must affect other people, given that it affects every single inline widget that tries to embed any custom HTML. I would dig into the bowels of the CKEditor engine myself, but I don't even have an initial pointer to where to look or what might cause it.

Is there any amount of money I can pay to fix this? I easily spent more than 20 hours on this, and would easily be happy to pay $1000 for a fix for this, since it continues to be blocking for actually rolling out our editor.

@neongreen
Copy link

I have stumbled upon the same (or similar?) problem. My inline widget turns into text when I type after it.

2021-01-02 21 54 18

@neongreen
Copy link

Funnily, after I recreate the editor from scratch (getData -> create new editor -> setData), already existing inline widgets work fine and don't change to plaintext. Only newly created widgets change to plaintext.

@neongreen
Copy link

Both the DOM and the model seem to be the same for the affected and the unaffected widget.

@neongreen
Copy link

I think the label pending:feedback should be removed — we have a reproduction case by @Discordius above.

@Mgsy
Copy link
Member

Mgsy commented Jan 4, 2021

Hi, I'm sorry for the late response. I'll take a look at this issue once again this week.

@Mgsy Mgsy removed the pending:feedback This issue is blocked by necessary feedback. label Jan 4, 2021
@Mgsy
Copy link
Member

Mgsy commented Jan 8, 2021

I was able to reproduce the problem with the code provided in #7358 (comment) - after pressing spacebar, the inline widget breaks.

After some investigation, I've noticed that this issue occurs only if  <span> in the initial HTML is empty:

<span class="math-tex"></span>

However, if you'll insert something to this <span>, e.g. &nbsp;, the inline widget behaves properly:

<span class="math-tex">&nbsp;</span>

@Discordius, could you please verify, whether inserting your element with &nbsp; inside (instead of empty <span>) fixes your issue?

@neongreen, is there a chance that you use similar markup in your case?

@Mgsy Mgsy added the pending:feedback This issue is blocked by necessary feedback. label Jan 8, 2021
@neongreen
Copy link

In my case, the <span> is not empty:

<span class="page-link ck-widget" data-id="aed58d06-3ee9-4bfa-920e-0337fbe1e048" contenteditable="false">
  <span class="page-link__react-wrapper">
    <svg ...></svg>
    <span>You need more slack</span>
  </span>
</span>

@neongreen
Copy link

I will post a self-contained example later today.

@Discordius
Copy link
Author

@Discordius, could you please verify, whether inserting your element with   inside (instead of empty ) fixes your issue?

As I mentioned above, it does make the bug disappear, but does not fix my issue, since I do not have control over whether the data gets inserted as empty vs. not. As I said above:

Sadly this doesn't really help me with fixing the bug when I edit the document, since that never routes through the data version of the document.

It doesn't matter whether the element has content in the model. And the bug appears without the content ever doing a roundtrip through the data-representation, so while this might help us find the reason for the bug, it doesn't highlight an obvious fix to me.

@Mgsy Mgsy added squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. and removed pending:feedback This issue is blocked by necessary feedback. labels Jan 11, 2021
@Mgsy Mgsy added support:3 An issue reported by a commercially licensed client. and removed support:2 An issue reported by a commercially licensed client. labels Jan 11, 2021
@niegowski
Copy link
Contributor

TL;DR

The issue should be solved if you downcast (in editing pipeline) your inline widget to the structure that would be matched by upcast conversion so like this:

conversion.for( 'upcast' )
	.elementToElement( {
		view: {
			name: 'span',
			classes: [ 'math-tex' ]
		},
		model: 'mathtex'
	} );

conversion.for( 'editingDowncast' )
	.elementToElement( {
		model: 'mathtex',
		view: ( modelItem, { writer } ) => toWidget( createMathTexEditingView( modelItem, writer, false ), writer )
	} );

conversion.for( 'dataDowncast' )
	.elementToElement( {
		model: 'mathtex',
		view: ( modelItem, { writer } ) => writer.createContainerElement( 'span', { class: 'math-tex' } )
	} );

function createMathTexEditingView( modelItem, writer ) {
	const mathTexView = writer.createContainerElement( 'span', { class: 'math-tex' } ); // <--- this matches matcher pattern from upcast conversion

	const uiElement = writer.createRawElement( 'span', { style: 'font-weight: bold' }, function( domElement ) {
		domElement.innerHTML = '[equation]';
		return domElement;
	} );

	writer.insert( writer.createPositionAt( mathTexView, 0 ), uiElement );

	return mathTexView;
}

So adding class attribute in the editing downcast to be matched by upcast conversion.

Details

Currently, CKEditor is detecting typing by observing document mutations, and for some cases, it tries to upcast the changed HTML to the model - this case is happening here, but in the editing conversion there is no class="math-tex" so the upcast conversion is not converting it and it's simply stripped by the default conversion.

@Reinmar Reinmar added this to the nice-to-have milestone Jan 18, 2021
@oleq oleq added the resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). label Jan 18, 2021
@oleq oleq modified the milestones: nice-to-have, iteration 39 Jan 18, 2021
@oleq oleq closed this as completed Jan 18, 2021
@Discordius
Copy link
Author

Huh, ok, this makes sense, but I think is also undocumented behavior?

I just went through all the docs on this that I could find, and it never once mentioned that anything from the editing-view would be upcast into the model. The full documentation talks about upcasting converters being responsible for translating the data-view into the model, not translating the editing-view layer into the model.

This seems like it's a really important set of constraints that should be communicated somewhere in the documentation. Indeed the documentation even very explicitly says:

It does not have its counterpart — there is no editing upcasting because all user actions are handled by editor features by listening to view events, analyzing what happened and applying necessary changes to the model. Hence, this process does not involve conversion.

But what we are dealing with here is exactly this! Editing-upcasting! Conversion in the editing pipeline! And you told me very explicitly this would not happen!

I am really confused about what is happening here. Does this mean my view and my data always have to follow the same set of constraints?

What if I want my editing view to look radically different from my data view?

Do all upcasting functions have to work on both editing-view-elements and data-view-elements? Is there any way to write upcasting handlers specifically for editing-view-elements but not data-view-elements and vice-versa?

@lslowikowska lslowikowska added support:1 An issue reported by a commercially licensed client. and removed support:3 An issue reported by a commercially licensed client. labels Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolution:invalid This issue is invalid (e.g. reports a non-existent bug or a by-design behavior). squad:core Issue to be handled by the Core team. support:1 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
Development

No branches or pull requests

8 participants