Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#13749 closed Bug (invalid)

Receive JavaScript errors when inserting Block level Placeholder

Reported by: Hugh Owned by:
Priority: Normal Milestone:
Component: UI : Widgets Version:
Keywords: Cc:

Description

Background

I'm trying to customize the Placeholder plugin to simply make it insert a Div instead of a Span. I am making minimal changes to the Placeholder plugin, and then I see Js errors in console with all "Insert" type commands when the editor does not have a visible caret.

Steps to reproduce

  1. Minimally customize plugins/placeholder/plugin.js as follows:
  • line 39: change the opening and closing tag from a span to a div
  • line 72: change to test for dtd.div instead of dtd.span
  • line 78: change from span to div
  1. Create a textarea with the content of only a placeholder, like this
    <textarea id="replaceme">[[sometag]]</textarea>
    
  2. CKEDITOR.replace(document.getElementById('replaceme'))
  3. Load the page and do not touch the editor area (do not set focus to it and make sure you cannot see the caret)
  4. Now click any of the toolbar "Insert" type commands (not necessarily Placeholder). Such as Insert Horizontal Line. or Insert Special Character.
  5. Receive one of several Javascript errors in console such as
    Uncaught TypeError: Cannot read property 'hasAscendant' of null
    Uncaught TypeError: Cannot read property 'checkReadOnly' of undefined
    

Expected result

Expect to be able to change Placeholder plugin minimally from Span container type to Div container type without causing Js errors.

Actual result

Getting errors when changing Placeholder type from span to div. I have not been able to track it down to anything I'm doing. When I stop it in the debugger everything looks correct at the end of the plugin's afterInit when it returns the transformed text.

Other details (browser, OS, CKEditor version, installed plugins)

When I do a fresh install, the plugin works fine (with the default container type of Span). I do only the changes above, and I get these errors. I do have the Widget plugin dependency. And Dialog. It was built with your download builder today which automatically added the dependencies. Testing on Google Chrome with version 4.5.3 freshly downloaded today. Have been trying to work through this for several hours on my own. Thanks for any help you can provide. I noticed that in the modified Placeholder plugin if I comment out the entire call to editor.dataProcessor.dataFilter.addRules I do not see the errors anymore (of course my source is not transformed to my desired div's as well). So I have isolated it to something that is going on there.

Change History (12)

comment:1 Changed 9 years ago by Hugh

here is my build config, so you could test with my exact setup: http://ckeditor.com/builder/544a1d22c6dfe8e23af36b1eee82633d

Last edited 9 years ago by Hugh (previous) (diff)

comment:2 Changed 9 years ago by Hugh

In step 3 of the steps to reproduce, if you do not have the placeholder in your global config, kindly change it to this instead:

CKEDITOR.replace(document.getElementById('replaceme'), { extraPlugins: 'placeholder'});

comment:3 Changed 9 years ago by Piotrek Koszuliński

Keywords: placeholder datafilter removed
Resolution: invalid
Status: newclosed
Version: 4.5.3

Hi! This would be a perfect bug report in terms of the quality of the description... if it didn't mention customising the official plugins ;). This makes things harder as we need to understand if what you did is correct.

For instance, I made the same modifications and I can see that loading data doesn't work as expected (widgets are not created). The reason for that is that xxx? text nodes are auto paragraphed and as paragraphs cannot contain divs, they aren't created. So it will be better to perform replacement (placeholder text to a div) before the content is parsed (and that would be in the editor#toHtml listener with a high priority - see the docs). This means that you also need to extend ACF (otherwise your divs will be filtered out). And this may be enough to fix the errors thrown upon insertion.

I'm closing this ticket as invalid for now. Unfortunately the more modifications you make, the more specific the bug report will need to be. We can reopen the ticket, but it must be more clear that it's the CKE working incorrectly and not your plugin.

comment:4 Changed 9 years ago by Hugh

Hi. Thanks for your help. I'm trying to find any sample code, that does anything with toHtml. I don't see any example of that. I'm not sure how to use it.

I think you understand, it is pretty simple what I want to do. I need to do exactly what the Placeholder plugin does, only use Div instead of Span. One small difference. Maybe it is not a bug, but my incorrect use of the API. If so, I hope you would take a few minutes to help me, since I spent over 3 hours of work yesterday trying to get these Spans into Divs, and still was not successful. I read a lot of the documentation, but since there aren't many examples, it is not clear how the pieces fit together.

I do not know what ACF is. (The abbreviation you used). Sorry, I'm new to CKEditor, but an expert Javascript developer. I just need some example of how to do the transformation, at the right time. I don't know what you mean by "extend ACF". I'm sorry. Can you point me to any example page that does this kind of transform or "extend"? So I could copy the code and adapt it to my needs? I looked at the documentation for toHtml and for HtmlDataProcessor, but I don't see this code actually being used so I don't know how these pieces are supposed to fit together.

Thanks so much for any help you can provide. I'm hopeful that I could work through this today, and then I would be happy to share my code with others for their benefit.

comment:5 Changed 9 years ago by Hugh

OK. I figured out ACF is Advanced Content Filter :)

But I still don't know how to "extend ACF". In the documentation I read about ACF it has some options for like "RequiredContent" and "ContentForms" but that does not seem applicable, since in my case, the content is not Tag, it is just String surrounded by brackets.

comment:6 Changed 9 years ago by Piotrek Koszuliński

See this: http://docs.ckeditor.com/#!/guide/plugin_sdk_integration_with_acf

Your data is a string, so if your plugin worked like the placeholder one, then you wouldn't need to do anything. But as I mentioned, in this case you will need to transform that text into divs before content is parsed, so before the ACF is applied. That means that you have to allow these divs.

comment:7 Changed 9 years ago by Hugh

I tried setting allowedContent=true. It does not help. I understand this disables ACF altogether. It still causes errors. Here is my new Plugin code:

( function() {
	CKEDITOR.plugins.add( 'placeholder', {
		requires: 'widget,dialog',
		lang: 'af,ar,bg,ca,cs,cy,da,de,el,en,en-gb,eo,es,et,eu,fa,fi,fr,fr-ca,gl,he,hr,hu,id,it,ja,km,ko,ku,lv,nb,nl,no,pl,pt,pt-br,ru,si,sk,sl,sq,sv,th,tr,tt,ug,uk,vi,zh,zh-cn', // %REMOVE_LINE_CORE%
		icons: 'placeholder', // %REMOVE_LINE_CORE%
		hidpi: true, // %REMOVE_LINE_CORE%

		onLoad: function() {
			// Register styles for placeholder widget frame.
			CKEDITOR.addCss( '.cke_placeholder{background-color: #FFFFD0;padding: 20px;margin-bottom: 10px;border: 1px solid #f5f5c0;border-radius: 5px;color: #55A611;font-weight: bold;font-size: larger;}' );
		},

		init: function( editor ) {

			var lang = editor.lang.placeholder;

			// Register dialog.
			CKEDITOR.dialog.add( 'placeholder', this.path + 'dialogs/placeholder.js' );

			// Put ur init code here.
			editor.widgets.add( 'placeholder', {
				// Widget code.
				dialog: 'placeholder',
				pathName: lang.pathName,
				// We need to have wrapping element, otherwise there are issues in
				// add dialog.
				template: '<div class="cke_placeholder">[[]]</div>',

				downcast: function() {
					return new CKEDITOR.htmlParser.text( '[[' + this.data.name + ']]' );
				},

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

				init: function() {
					// Note that placeholder markup characters are stripped for the name.
					this.setData( 'name', this.element.getText().slice( 2, -2 ) );
				},

				data: function() {
					this.element.setText( '[[' + this.data.name + ']]' );
				}
			} );

			editor.ui.addButton && editor.ui.addButton( 'CreatePlaceholder', {
				label: lang.toolbar,
				command: 'placeholder',
				toolbar: 'others,5',
				icon: 'placeholder'
			} );

            var placeholderReplaceRegex = /\[\[([^\[\]])+\]\]/g;
            editor.on( 'toHtml', function( evt ) {
                        var evtData = evt.data,
                            text = evtData.dataValue;

                evtData.dataValue = text.replace( placeholderReplaceRegex, function( match ) {
						// Creating widget code.
						var widgetWrapper = null,
							innerElement = new CKEDITOR.htmlParser.element( 'div', {
								'class': 'cke_placeholder'
							} );

						// Adds placeholder identifier as innertext.
						innerElement.add( new CKEDITOR.htmlParser.text( match ) );
						widgetWrapper = editor.widgets.wrapElement( innerElement, 'placeholder' );

						// Return outerhtml of widget wrapper so it will be placed
						// as replacement.
						return widgetWrapper.getOuterHtml();
					} );
            }, null, null, 1 );
		},
	} );

} )();

As you can see, now I am using toHtml event instead of afterInit. It doesn't seem to matter. Here is my config:

        var ckEditorConfig = {
            allowedContent: true,
            enterMode: CKEDITOR.ENTER_BR,
            forcePasteAsPlainText: false,
            height: '50vh',
            extraPlugins: 'placeholder'
        };
        var textarea = $('#ReportLayout').get(0);
        CKEDITOR.replace(textarea, ckEditorConfig);

I still have the same problem, even though ACF is disabled here. Insert toolbar buttons cause JS errors.

If I remove the widget wrapper line, it works if I use this:

						return innerElement.getOuterHtml();

Instead of this:

						widgetWrapper = editor.widgets.wrapElement( innerElement, 'placeholder' );

						// Return outerhtml of widget wrapper so it will be placed
						// as replacement.
						return widgetWrapper.getOuterHtml();

But now my Placeholder contents are editable, which is not what I want. They need to be non-editable, like a Widget.

comment:8 in reply to:  6 Changed 9 years ago by Hugh

Replying to Reinmar:

Bro. are u still there. This looks like a bug with widget wrapper to me. It's why I opened up this ticket. What do I need to do to make it properly init with the Block level elements? I did exactly what you told me but it still has the same failure with unrelated Insert tools.

We need these block level elements. They are placeholders. This app is to build a Template for a report. These report building blocks get added as placeholders. They could turn into a chart, or a big area of data that takes up half a page. So the placeholder needs to be a big, full size square. Block level. But what's great is in the editor, they can type what they want, above and below, all around the placeholders. Then later on server side we generate the report by substituting the other database driven features into the report template where the placeholders were.

We're real close here! Is there something else I need to do to make it recognize the Divs as widgets in the content? As you can see in my code, I attached the toHtml handler with priority 1, so I thought later in the process, that should properly turn into a widget, wouldn't you say? And can you confirm that my understanding is correct, that by setting allowedContent=true it should not involve ACF so we can eliminate that from being the issue?

Thanks for your help kind sir :)

comment:9 Changed 9 years ago by Piotrek Koszuliński

I used the code that you posted and I can't see any errors. Are you sure that errors are caused by this plugin and not by other ones? I noticed now that you wrote:

Now click any of the toolbar "Insert" type commands (not necessarily Placeholder). Such as Insert Horizontal Line. or Insert Special Character.

I haven't noticed this previously. That rather means that it's not your modification in the placeholder plugin, but something else. I can't reproduce it however, so you will need to create a sample (e.g. on jsfiddle) on which it can be reproduced.

comment:10 Changed 9 years ago by Hugh

Here is a JSFiddle that shows the error. http://jsfiddle.net/HugeHugh/3xu8yece/

All you must do, is load the JSFiddle, and press the Insert Horizontal Line button. It causes the JS Error. Attempting to insert a Special Character, causes a different JS Error. Be sure to not set focus to the Editor before pressing the Insert buttons.

It is a fork of this JSFiddle I found: http://jsfiddle.net/oleq/M2YLe/

It only happens if the Div placeholder is the first thing in the textarea.

Thank you Sir for your continued support.

comment:11 in reply to:  9 Changed 9 years ago by Hugh

Replying to Reinmar:

Hi Reinmar :) I was hoping to hear back from you, since I shared the JSFiddle that displays the error, as you had requested.

As you can see, the JSFiddle doesn't try to do any transformation on the source text, and it still has the issue. So I think this helps us narrow it down. The Fiddle also has a rule for ACF, so that could be ruled out as well.

If you simply rearrange the html inside the textarea to not begin with a <div>, notice how it works fine, and doesn't have any JavaScript error. So I think this also helps to narrow down the test cases for this issue.

If you now consider the issue valid, can we please re-open my ticket? Or, should I open a new ticket that has only this JSFiddle that causes the error?

comment:12 Changed 9 years ago by Piotrek Koszuliński

Hi HDogger. So I can see that it's just a matter of having a block widget at the beginning of the editable. It can be any block widget. Since this ticket already contains a lot of discussions about unrelated things, I guess it would be best to start with a fresh one – #13778.

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy