Opened 6 years ago

Closed 5 years ago

#16482 closed defect (fixed)

Regression: _WidgetBase#placeAt() can't be used on a DocumentFragment

Reported by: Bryan Forbes Owned by: Bryan Forbes
Priority: high Milestone: 1.10
Component: Dijit Version: 1.8.0
Keywords: Cc: ben hockey
Blocked By: Blocking:

Description

var frag = document.createDocumentFragment();
widget.placeAt(frag);

This will fail because reference.tagName is checked in placeAt which causes refWidget to be set to the document fragment. In the else condition, ref is set to undefined because refWidget is truthy and both containerNode and domNode are undefined on the fragment. This should be checking for appendChild or nodeType rather than tagName. This is also a regression because this used to work in 1.7.

Change History (13)

comment:1 Changed 6 years ago by Bryan Forbes

Component: GeneralDijit
Milestone: tbd1.8.3
Owner: set to bill
Priority: undecidedhigh
Status: newassigned

comment:2 Changed 6 years ago by ben hockey

Cc: ben hockey added

comment:3 Changed 6 years ago by Bryan Forbes

This broke as part of the change in [28251].

comment:4 Changed 6 years ago by bill

Owner: changed from bill to Bryan Forbes
Status: assignedpending
Version: 1.8.2rc11.8.0

Hmm, placeAt() isn't documented to be able to work on a DocumentFragment. The API doc says it takes a Widget, DOMNode, or id of widget or DOMNode. I guess you are saying you want _WidgetBase.placeAt(docFrag) it to call through to domConstruct.place(), although that's also not documented to work with a DocumentFragment.

This will fail because reference.tagName is checked in placeAt which causes refWidget to be set to the document fragment.

I'm not following you there. The code checking tagName is:

var refWidget = !reference.tagName && registry.byId(reference);

Regardless of the first clause in the expression, refWidget will be null (or undefined), because registry.byId(frag) will return nothing.

Version 0, edited 6 years ago by bill (next)

comment:5 Changed 6 years ago by Bryan Forbes

Status: pendingnew
		byId: function(/*String|Widget*/ id){
			// summary:
			//		Find a widget by it's id.
			//		If passed a widget then just returns the widget.
			return typeof id == "string" ? hash[id] : id;	// dijit/_WidgetBase
		},

Since id is not a string (it's a DocumentFragment), it is returned. The same would happen with a DOMNode if not for the tagName check.

comment:6 Changed 6 years ago by bill

Status: newassigned

Funny how registry.byId() can return something other than a widget.

Anyway, I talked to Bryan about this over IM. If he adds DOH tests for dojo.place() and _WidgetBase.placeAt() to work with a DocumentFragment target (in addition to an Element target), and updates the API docs to say that a DocumentFragment is also allowed, I'm OK with the suggested change.

Apparently, a DocumentFragment target is supposed to work exactly the same as an unattached Element target: the position parameter can be "first", "last"/undefined, or a number, but not "before", "after", or "replace".

comment:7 Changed 6 years ago by Kitson Kelly

Milestone: 1.8.31.8.4

comment:8 Changed 6 years ago by Colin Snover

Milestone: 1.8.41.8.5

Moving incomplete issues to next minor release.

comment:9 Changed 6 years ago by Colin Snover

Milestone: 1.8.51.8.6

1.8.5 is released; moving all outstanding tickets to next milestone.

comment:10 Changed 5 years ago by Colin Snover

Milestone: 1.8.61.8.7

1.8.6 is released; moving all outstanding tickets to next milestone.

comment:11 Changed 5 years ago by Bill Keese <bill@…>

In 2f1094bcf75cdc0ff58e3ce76bbedb8e9228bd41/dojo:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:12 Changed 5 years ago by bill

Milestone: 1.8.71.10

comment:13 Changed 5 years ago by Bill Keese <bill@…>

Resolution: fixed
Status: assignedclosed

In 38230b8c473fa1be0410366d9454332c5f38d44e/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 
Note: See TracTickets for help on using tickets.