Opened 14 years ago

Last modified 11 years ago

#6298 closed defect

[patch][cla]bugs in Editor/RichText widget programmatic creation — at Version 5

Reported by: [email protected] Owned by: liucougar
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 (5)

comment:1 Changed 14 years ago by Adam Peller

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

comment:2 Changed 14 years ago by Adam Peller

Description: modified (diff)

comment:3 Changed 14 years ago by Adam Peller

Milestone: 1.11.1.1

we need a patch file and a test case

comment:4 Changed 14 years ago by Adam Peller

Reporter: changed from Chris Mitchell to [email protected]

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 14 years ago by Adam Peller

Component: GeneralEditor
Description: modified (diff)
Owner: changed from anonymous to liucougar
Note: See TracTickets for help on using tickets.