Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#15567 closed defect (invalid)

Inconsistent use of "id" vs." widgetId" in registry

Reported by: Randy Hudson Owned by: bill
Priority: undecided Milestone: tbd
Component: Dijit Version: 1.7.3
Keywords: Cc:
Blocked By: Blocking:

Description

In registry.js (previously in manager.js) functions inconsistently use either the "id" property or "widgetId" dom attribute. For example, registry add and remove use the widget's id property, but getEnclosingWidget expects the dom node to have a widgetId attribute.

As a result of this inconsistent use of names, when a nested, templated widget calls getParent() from postCreate, it fails to return the parent even though the parent has already been added to the registry. This is because while WidgetBase#create?() registers itself before the templated child is constructed, it doesn't bother to set the redundant widgetId property until much later (after buildRendering).

Change History (4)

comment:1 Changed 7 years ago by bill

Resolution: invalid
Status: newclosed

I don't see it as inconsistent. If you have a widget and you want the id, you can simply access widget.id, but if you have a DOMNode and you want to map it to a widget, you can't look at widget.id because you don't have a pointer to widget. How else could it be coded?

It is true that you can't call getParent() from postCreate() in a nested widget. However, you should not be doing that anyway. postCreate() for any widget should not assume that the widget is in the DOM, because this will fail:

new MyWidget()
Last edited 7 years ago by bill (previous) (diff)

comment:2 in reply to:  1 ; Changed 7 years ago by Randy Hudson

Replying to bill:

I don't see it as inconsistent. If you have a widget and you want the id, you can simply access widget.id, but if you have a DOMNode and you want to map it to a widget, you can't look at widget.id because you don't have a pointer to widget. How else could it be coded?

Obviously one way it could be coded is for both the widget and dom node to use either "id" or "widgetId" consistently.

In my case, the widget is always told what its ID should be in the initial HTML. This means that I'm setting the dom's "id" attribute, which then gets copied over into the widget's id. The nested widget is always contained by the parent, so it can always call getParent().

This is clearly a bug. It is a hack for me to have to generate the HTML and set both the id and widgetId attributes on the same element to the same value. If a widget's ID is reflected in the DOM as the "widgetId" attribute, then I should just be able to set ONLY the "widgetId" attribute on a DIV and BaseWidget? would pull its ID property from that attribute.

It is true that you can't call getParent() from postCreate() in a nested widget. However, you should not be doing that anyway. postCreate() for any widget should not assume that the widget is in the DOM, because this will fail:

Your logic is bogus. Of course a widget might not always have a parent, but when it DOES, calling getParent() should work.

The safe fix here is pretty obvious. Move this code:

if(this.domNode){

Note: for 2.0 may want to rename widgetId to dojo._scopeName + "_widgetId", assuming that dojo._scopeName even exists in 2.0 this.domNode.setAttribute("widgetId", this.id);

}

above this code:

dijit.registry.add(this);

comment:3 in reply to:  2 Changed 7 years ago by bill

Replying to randyhudson:

Obviously one way it could be coded is for both the widget and dom node to use either "id" or "widgetId" consistently.

Yes, we could switch everything to use widgetId, but I find new MyWidget({ widgetId: "foo"}) to be less intuitive and less pleasant than the current syntax of new MyWidget({ id: "foo"}).

Even more so for <input data-dojo-type=dijit/form/TextBox data-dojo-props="widgetId: foo"> instead of <input data-dojo-type=dijit/form/TextBox id="foo">

You also need to consider the markup generated for something like a ValidationTextBox, it becomes

<div widgetId=foo>
   ...
      <input id=foo>
</div>

For accessibility etc. reasons, the id needs to be set on the <input> (the focus node) as the attribute named "id". Thus the only choice for the root DOMNode attribute name is "widgetId".

The safe fix here is pretty obvious. Move this code:

if(this.domNode){

Note: for 2.0 may want to rename widgetId to dojo._scopeName + "_widgetId", assuming that dojo._scopeName even exists in 2.0 this.domNode.setAttribute("widgetId", this.id);

}

above this code:

dijit.registry.add(this);

No, that won't work because this.domNode doesn't exist until after the call to buildRendering(), and buildRendering() isn't called until after registry.add().

comment:4 Changed 7 years ago by Randy Hudson

The code would change to check for srcNodeRef and setting the widgetId attribute on it instead of domNode.

(We actually do templating on the server and have replaced buildRendering to simply hook up with the server-built rendering)

Note: See TracTickets for help on using tickets.