Opened 10 years ago

Closed 10 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 10 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 10 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 10 years ago.
Patch to add test to ContentPane.html
12348test.2.diff (1.1 KB) - added by Kenneth G. Franqueiro 10 years ago.
Making test moar betterer

Download all attachments as: .zip

Change History (8)

Changed 10 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 10 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 10 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 10 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 10 years ago by Kenneth G. Franqueiro

Attachment: 12348test.diff added

Patch to add test to ContentPane.html

Changed 10 years ago by Kenneth G. Franqueiro

Attachment: 12348test.2.diff added

Making test moar betterer

comment:3 Changed 10 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 10 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.