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

widget startupData object is not being used by widget template #3540

Closed
spacevoyager78 opened this issue Oct 4, 2019 · 4 comments · Fixed by #5408
Closed

widget startupData object is not being used by widget template #3540

spacevoyager78 opened this issue Oct 4, 2019 · 4 comments · Fixed by #5408
Assignees
Labels
plugin:widget The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:feature A feature request.

Comments

@spacevoyager78
Copy link

spacevoyager78 commented Oct 4, 2019

Accorrding to the widget docs, I can use:

editor.execCommand( 'simplebox', {
    startupData: {
        align: 'left'
    }
} );

to create a widget with initial widget data and this data object will overwrite the default data.

However, this doesn't modify the widget template at all.

I have created an example here: http://users.sch.gr/vandr/ckeditor4/

I use unoptimized CKEditor 4.13.0 with simplebox widget.
I have modified the simplebox widget by including "title" and "content" placeholders like so:

template:
    '<div class="simplebox">' +
    '<h2 class="simplebox-title">{title}</h2>' +
    '<div class="simplebox-content"><p>{content}</p></div>' +
    '</div>',

and I have registered another copy named simplebox2 widget with defaults object:

defaults: {
    title: 'default title',
    content: 'default content'
},

The plugin source is here: http://users.sch.gr/vandr/ckeditor4/plugins/simplebox/plugin.js
I created CKEditor like this:

var editor1 = CKEDITOR.replace('editor1', {
	extraPlugins: 'simplebox',
	on: {
		instanceReady: function(evt) {
			evt.editor.execCommand('simplebox2', {
				startupData: {
					title: 'startupData title',
					content: 'startupData content'
				}
			});
		},
		save: function(evt) {
			evt.cancel();
			evt.editor.execCommand('simplebox', {
				startupData: {
					title: 'startupData title',
					content: 'startupData content'
				}
			});
		}
	}
});

Initially, simplebox2 widget is inserted (which has defaults object), but startupData is wrongly never used in the template and the defaults object is used.
If you press Save to insert simplebox widget (which doesn't have defaults object), startupData is also wrongly not used and I get error:

template.js:64 Uncaught TypeError: Cannot read property 'title' of undefined
at template.js:64
at String.replace ()
at CKEDITOR.template.output (template.js:63)
at CKEDITOR.command.exec (plugin.js:1990)
at CKEDITOR.command.exec (command.js:56)
at Editor.execCommand (editor.js:982)
at Editor.save ((index):22)
at Editor.listenerFirer (event.js:144)
at Editor.CKEDITOR.event.fire (event.js:290)
at Editor.CKEDITOR.editor.fire (editor_basic.js:24)

So, I cannot use execCommand with startupData to create a widget and modify its template. I also found an old question in StackOverflow: https://stackoverflow.com/questions/32149518/ckeditor-widget-receives-data-after-it-has-been-rendered

@spacevoyager78 spacevoyager78 added the type:bug A bug. label Oct 4, 2019
@spacevoyager78 spacevoyager78 changed the title widget startupData doesn't modigy widget template widget startupData doesn't modify widget template Oct 4, 2019
@spacevoyager78 spacevoyager78 changed the title widget startupData doesn't modify widget template widget startupData object doesn't modify widget template Oct 4, 2019
@spacevoyager78 spacevoyager78 changed the title widget startupData object doesn't modify widget template widget startupData object doesn't modify widget template because it's undefined Oct 4, 2019
@spacevoyager78
Copy link
Author

spacevoyager78 commented Oct 4, 2019

I have found a solution, but I don't how to make a PR, so I'll put here, so someone could test it, please!
Basically, in line 1990 from plugins\widget\plugin.js
widgetDef.template.output( defaults )
has to be modified to
widgetDef.template.output( { ...defaults, ...commandData.startupData } )
or
widgetDef.template.output( Object.assign( {}, defaults, commandData.startupData ) )
or perhaps use some code to support Internet Explorer like this

var mergedData = Object( defaults );
for ( var key in commandData.startupData ) {
	if ( commandData.startupData.hasOwnProperty( key ) ) {
		mergedData[ key ] = commandData.startupData[ key ];
	}
}
widgetDef.template.output( mergedData )

In conclusion, the easy fix is to merge defaults object with commandData.startupData object, with the latter overriding the former, just before outputting the template..

@spacevoyager78 spacevoyager78 changed the title widget startupData object doesn't modify widget template because it's undefined widget startupData object is not being used by widget template Oct 6, 2019
@jacekbogdanski jacekbogdanski self-assigned this Oct 7, 2019
@jacekbogdanski
Copy link
Member

jacekbogdanski commented Oct 7, 2019

I can reproduce the issue using your sample. To simplify it a bit, here are reproduction steps for the issue:

  1. Execute below code to initialize CKEditor:
CKEDITOR.plugins.add( 'simplebox', {
	requires: 'widget',
	init: function( editor ) {
		editor.widgets.add( 'simplebox', {
			allowedContent: 'div(!simplebox); div(!simplebox-content); h2(!simplebox-title)',

			requiredContent: 'div(simplebox)',

			editables: {
				title: {
					selector: '.simplebox-title',
					allowedContent: 'br strong em'
				},
				content: {
					selector: '.simplebox-content',
					allowedContent: 'p br ul ol li strong em'
				}
			},

			template:
			'<div class="simplebox">' +
			'<h2 class="simplebox-title">{title}</h2>' +
			'<div class="simplebox-content"><p>{content}</p></div>' +
			'</div>',

			upcast: function( element ) {
				return element.name == 'div' && element.hasClass( 'simplebox' );
			},

			defaults: {
				title: 'default title',
				content: 'default content'
			},
		} );
	}
} );

CKEDITOR.replace( 'editor', {
	extraPlugins: 'simplebox'
} );
  1. Focus the editor.
  2. Execute
editor.execCommand( 'simplebox', {
  startupData: {
    title: 'updated title',
    content: 'updated content'
  }
} );

Expected

Widget should be initialized with updated placeholders i.e updated title and updated content

Actual

Widget is initialized with default values.

Overall, there is no strict information in our docs that widgetDef.startupData will be used to populate the template. However, as widgetDef.defaults is used for this purpose and there is information about
higher startupData priority over defaults, it probably makes sense to unify this behavior.

However, it seems for me more like a feature than a bug as there is no well-written specification for that (at least I couldn't find) and the whole concept is rather based on assumptions than docs.

@jacekbogdanski jacekbogdanski added status:confirmed An issue confirmed by the development team. type:feature A feature request. plugin:widget The plugin which probably causes the issue. and removed type:bug A bug. labels Oct 7, 2019
@jacekbogdanski jacekbogdanski removed their assignment Oct 7, 2019
@spacevoyager78
Copy link
Author

Yes, you are right, there is no direct information that startupData is used in the template. It's what I inferred, based on the fact that the template uses default data and startupData overrides it. I think it's very useful to have it and the fix seems (to me at least) rather easy. Then again, I can't speak for the whole internal structure of widget plugin, that's why I believe it's best to leave it for the devs. Temporarily, I'm using my fix and I'm happy with it.

@bricas
Copy link

bricas commented Sep 15, 2022

I realize this ticket is a bit old now, but, is there a solid work-around that doesn't involve the patch?

@jacekbogdanski jacekbogdanski self-assigned this Nov 9, 2022
@jacekbogdanski jacekbogdanski removed their assignment Nov 28, 2022
@Comandeer Comandeer self-assigned this Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin:widget The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:feature A feature request.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants