Ticket #6298 (new defect)

Opened 8 months ago

Last modified 2 months ago

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

Reported by: chrism1@… Owned by: liucougar
Priority: normal Milestone: 1.4
Component: Editor Version: 1.1b1
Severity: normal Keywords:
Cc:

Description (last modified by peller) (diff)

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

Changed 8 months ago by peller

  • reporter changed from guest to chrism
  • description modified (diff)
  • milestone set to 1.1

Changed 8 months ago by peller

  • description modified (diff)

Changed 8 months ago by peller

  • milestone changed from 1.1 to 1.1.1

we need a patch file and a test case

Changed 8 months ago by peller

  • reporter changed from chrism to chrism1@us.ibm.com

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.

Changed 8 months ago by peller

  • owner changed from anonymous to liucougar
  • component changed from General to Editor
  • description modified (diff)

Changed 8 months ago by bill

  • milestone changed from 1.1.1 to 1.2

Changed 3 months ago by bill

  • milestone changed from 1.2 to 1.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.

Changed 2 months 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().

Note: See TracTickets for help on using tickets.