Opened 11 years ago

Closed 9 years ago

#6298 closed defect (wontfix)

[patch][cla]bugs in Editor/RichText widget programmatic creation

Reported by: chrism1@… Owned by:
Priority: high Milestone: future
Component: Editor Version: 1.1b1
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by Adam Peller)

When creating Editor instances programmatically (IDE use case), there is a that keeps RichText from being properly initialized when the widget is not yet attached to DOM.

This problem exists in trunk (1.1rc1)

Here is a summary of the patches necessary to fix the problem:

The first problem is that the the RichText.open function calls dojo.place(...,"before"), but in our scenario the refNode does not yet have a parent node... so place can default to just appendChild as follows:

(rev 13182) dojo/_base/html.js: (see <<< inline)
	dojo.place = function(/*String|DomNode*/node, /*String|DomNode*/refNode, /*String|Number*/position){
		//	summary:
		//		attempt to insert node in relation to ref based on position
		//	node: 
		//		id or reference to node to place relative to refNode
		//	refNode: 
		//		id or reference of node to use as basis for placement
		//	position:
		//		string noting the position of node relative to refNode or a
		//		number indicating the location in the childNodes collection of
		//		refNode. Accepted string values are:
		//			* before
		//			* after
		//			* first
		//			* last
		//		"first" and "last" indicate positions as children of refNode.

		// FIXME: need to write tests for this!!!!
		if(!node || !refNode || position === undefined){ 
			return false;	//	boolean 
		}
		node = dojo.byId(node);
		refNode = dojo.byId(refNode);
		if(refNode.parentNode){ // <<< Guard against case when node not yet attached to DOM
			if(typeof position == "number"){
				var cn = refNode.childNodes;
				if((position == 0 && cn.length == 0) ||
					cn.length == position){
					refNode.appendChild(node); return true;
				}
				if(position == 0){
					return _insertBefore(node, refNode.firstChild);
				}
				return _insertAfter(node, cn[position-1]);
			}
			switch(position.toLowerCase()){
				case "before":
					return _insertBefore(node, refNode);	//	boolean
				case "after":
					return _insertAfter(node, refNode);		//	boolean
				case "first":
					if(refNode.firstChild){
						return _insertBefore(node, refNode.firstChild);	//	boolean
					}
					// else fallthrough...
				default: // aka: last
					refNode.appendChild(node);
					return true;	//	boolean
			}
		}else{
			refNode.appendChild(node);
			return true;
		}
	}

The other place that needs to be fixed is in (r13182)RichText?._drawIFrame, need to test contentDoc being null...

...

		var contentDoc = this.iframe.contentDocument;
		if (contentDoc){ // possible if RichText created but not yet attached to DOM
			contentDoc.open();
			if(dojo.isAIR){
				contentDoc.body.innerHTML = html;
			}else{
				contentDoc.write(this._getIframeDocTxt(html));
			}
			contentDoc.close();
		}
...

With these two mods, the Editor now works properly in IDE use case. These patches are submitted under ICLA.

-Chris Mitchell, IBM

Change History (10)

comment:1 Changed 11 years ago by Adam Peller

Description: modified (diff)
Milestone: 1.1
Reporter: changed from guest to Chris Mitchell

comment:2 Changed 11 years ago by Adam Peller

Description: modified (diff)

comment:3 Changed 11 years ago by Adam Peller

Milestone: 1.11.1.1

we need a patch file and a test case

comment:4 Changed 11 years ago by Adam Peller

Reporter: changed from Chris Mitchell to chrism1@…

I'd prefer we not change the behavior of dojo.place, but instead modify the call from RichText?.open. I'm concerned about changing the relationship of those two nodes (domNode/textarea) from sibling to parent/child. Is that ok?

Also, skipping the write() call is problematic in a few ways. First off, the caller needs to call open again once the editor is added to the DOM in order for content to appear. We could use the startup pattern used by layout widgets, I guess.

The other problem is that an exception is thrown where this.document.body is checked (this.document is null) and it's explicitly checked with a throw.

Why is this needed again? Is it just for completeness? Could the editor be created on an existing DOM node, like in the current test_Editor.html example? Apparently, iframes aren't happy if they aren't in the DOM -- no contentDocument.

comment:5 Changed 11 years ago by Adam Peller

Component: GeneralEditor
Description: modified (diff)
Owner: changed from anonymous to liucougar

comment:6 Changed 11 years ago by bill

Milestone: 1.1.11.2

comment:7 Changed 11 years ago by bill

Milestone: 1.21.4

As peller said, this is more of a limitation than a bug. Admittedly would be nice if it works but there's a clear workaround as shown in test_Editor.html.

comment:8 Changed 11 years ago by bill

FYI, another reason Editor requires a srcNodeRef is because it copies the style from the srcNodeRef to the iframe document, see RichText's _getIframeDocTxt().

comment:9 Changed 10 years ago by bill

Milestone: 1.4future
Owner: liucougar deleted

comment:10 Changed 9 years ago by Jared Jurkiewicz

Resolution: wontfix
Status: newclosed

There are too many issues with trying to create the RTE without having a dom node in the document to begin with. A lot of them are limitations of how iframes operate. while it would be nice to fix this, I don't see it as possible in the forseeable future and there are numerous ways to fade in an editor to make the startup look cleaner.

So, closing this for now since I do not expect this to get resolved.

Note: See TracTickets for help on using tickets.