Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#17132 closed defect (fixed)

[CLA][patch] _CssStateMixin throws an unhandled exception from focus change handler

Reported by: MaxMotovilov Owned by: bill
Priority: undecided Milestone: 1.9.1
Component: Dijit Version: 1.9.0
Keywords: Cc:
Blocked By: Blocking:

Description

The code in focus change handler assumes that any node with _cssState attribute set would have an enclosing widget. This may not be the case when widgets are destroyed dynamically; a simple check should solve the issue (see patch).

Attachments (1)

csstate-focus-exception.patch (464 bytes) - added by MaxMotovilov 7 years ago.

Download all attachments as: .zip

Change History (6)

Changed 7 years ago by MaxMotovilov

comment:1 Changed 7 years ago by bill

Owner: changed from bill to MaxMotovilov
Status: newpending

Seems like if a widget is destroyed (dynamically or otherwise) the node with _cssState attribute should be destroyed too. Can you give a test case where this doesn't happen?

comment:2 Changed 7 years ago by MaxMotovilov

Status: pendingnew

This happens when the widget is destroyed with .destroy(true) and the DOM fragment it resides in is summarily overwritten (and presumably garbage collected). Which leads me to a thought that .destroy(true) does not necessarily disconnect all of the event handlers -- would that be a bug?

comment:3 Changed 7 years ago by bill

Milestone: tbd1.9.1
Owner: changed from MaxMotovilov to bill
Status: newassigned

OK... well your patch will certainly fix the exception in that case.

Really though if the widget is destroyed with destroy(true), the _cssState properties on its DOMNode and its descendant DOMNodes should be removed. Otherwise you'll have problems in the case of nested widgets:

<div data-dojo-type=... data-dojo-id=outer>
      <div data-dojo-type ... data-dojo-id=inner>
            <span id=nodeWithCssStateSet>...

If you call inner.destroy(true), then a mouse over nodeWithCssStateSet will send the notification to outer, specifically calling outer._subnodeCssMouseEvent(). I guess that wouldn't cause an exception but technically it's not the correct behavior.

For 2.0 though I'd like to get rid of _cssState property either altogether, or at least on widget subnodes, so the simple solution to avoid the exception seems fine for now.

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

comment:4 Changed 7 years ago by Bill Keese

Resolution: fixed
Status: assignedclosed

avoid null pointer exception when _Widget.destroy(true) call, fixes #17132, thanks Max

Changeset: 14eef249793df93118e92d31eb2ad55e107b3a1f

comment:5 Changed 7 years ago by Bill Keese

avoid null pointer exception when _Widget.destroy(true) call, fixes #17132, thanks Max

Changeset: 518ece0d690335764183f12503cb064291d5f5dc

Note: See TracTickets for help on using tickets.