Opened 9 years ago

Closed 9 years ago

#12348 closed defect (fixed)

ContentPane: potential to double-parse when setting value to a widget instance

Reported by: Kenneth G. Franqueiro Owned by: Kenneth G. Franqueiro
Priority: high Milestone: 1.7
Component: Dijit Version: 1.5
Keywords: Cc:
Blocked By: Blocking:

Description

If parseOnLoad is true on a dijit.layout.ContentPane instance and you set its value to a widget instance, the parser will still attempt to run on the content of the passed widget's DOM node. If the passed widget happens to have been a templated widget containing non-templated widgets, some nodes may still have lingering dojoTypes which the parser will then incorrectly attempt to instantiate into widgets, potentially causing id errors.

Attachments (4)

testCPparse.html (698 bytes) - added by Kenneth G. Franqueiro 9 years ago.
simple test that will produce an error due to conflicting id to demonstrate the problem. This example is simplistic and contrived but it's certainly probable for someone to have a ContentPane? that contains a non-templated widget (layout or otherwise) within.
12348.diff (495 bytes) - added by Kenneth G. Franqueiro 9 years ago.
simple fix that will only parse if parseOnLoad is true *and* the passed content is *not* already a widget.
12348test.diff (1.1 KB) - added by Kenneth G. Franqueiro 9 years ago.
Patch to add test to ContentPane.html
12348test.2.diff (1.1 KB) - added by Kenneth G. Franqueiro 9 years ago.
Making test moar betterer

Download all attachments as: .zip

Change History (8)

Changed 9 years ago by Kenneth G. Franqueiro

Attachment: testCPparse.html added

simple test that will produce an error due to conflicting id to demonstrate the problem. This example is simplistic and contrived but it's certainly probable for someone to have a ContentPane? that contains a non-templated widget (layout or otherwise) within.

Changed 9 years ago by Kenneth G. Franqueiro

Attachment: 12348.diff added

simple fix that will only parse if parseOnLoad is true *and* the passed content is *not* already a widget.

comment:1 Changed 9 years ago by Kenneth G. Franqueiro

Version: 1.6.0rc11.5

Forgot to mark the version correctly. Actually this happens even further back than 1.5, but just to be clear, it's *not* a 1.6 regression.

comment:2 Changed 9 years ago by bill

The fix looks fine to me but you need to add an automated test case (probably in ContentPane.html). Not sure if we have any tests for passing in a widget to set("content", ... ) , or as a parameter to the constructor. We should though.

Changed 9 years ago by Kenneth G. Franqueiro

Attachment: 12348test.diff added

Patch to add test to ContentPane.html

Changed 9 years ago by Kenneth G. Franqueiro

Attachment: 12348test.2.diff added

Making test moar betterer

comment:3 Changed 9 years ago by bill

Milestone: tbd1.7
Owner: set to Kenneth G. Franqueiro

Looks good to me, feel free to check in after the 1.6 freeze.

comment:4 Changed 9 years ago by Kenneth G. Franqueiro

Resolution: fixed
Status: newclosed

(In [23950]) Add check to ContentPane? to ensure parser is not run if content is set to a widget; tests included. Fixes #12348

Note: See TracTickets for help on using tickets.