Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#16785 closed defect (fixed)

Tree: throws invalid PathError on startup

Reported by: Peter Jekel Owned by: bill
Priority: undecided Milestone: 1.9
Component: Dijit Version: 1.9.0a2
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by bill)

The dijit/Tree throws an invalid 'PathError' whenever two different trees use, or get, the same ID.

The scenario:

First - The user runs application A which creates a Tree using the DIV name 'myTree'. He then expands and selects any of the child nodes.

Next - The user runs a different application B which also creates a Tree using the DIV name 'myTree' but this tree uses a different data store. As a result, the tree throws a PathError? just because it can't find the item that was selected in the tree in application A.

This scenario implies that the tree ID must not only be unique within an application but also across applications.

I don't understand the added value of throwing the PathError? or why this behavour has changed from dojo 1.8. In addition, preserving info about what nodes have been selected between sessions can also be questioned, I can't find any use case for it.

A better approach with regards to cookies would be to have the 'persist' property default to false. If the user want to preserve the tree state between session he/she must explicitly set the persist property AND provide a cookie name instead of have the cookie name generated based on an arbitrary ID that may or may not be assigned by dijit.

I have attached two simple apps A and B to demonstrate the effect. Run app A first, expand the tree and select a node. Close app A and run app B.

Attachments (2)

test_A.html (1.6 KB) - added by Peter Jekel 7 years ago.
test_B.html (1.5 KB) - added by Peter Jekel 7 years ago.

Download all attachments as: .zip

Change History (9)

Changed 7 years ago by Peter Jekel

Attachment: test_A.html added

Changed 7 years ago by Peter Jekel

Attachment: test_B.html added

comment:1 Changed 7 years ago by bill

Description: modified (diff)
Milestone: tbd1.9
Status: newassigned
Summary: Diji/Tree throws invalid PathErrorTree: throws invalid PathError on startup

So, even though the URL is different, it uses the same cookie? Even if it's the same URL though, I've seen errors because the Tree data has changed, and the previously selected node no longer exists.

I agree that persist should be false by default.

The code to persist selection was added in #14058, but now that you mention it, I don't see a purpose to it either. It was probably added more to remember the last focused node rather than the last selected node, but that's also misguided since Tree.focus() by default does not associate focus with selection.

About PathError, it's natural for set("path", ...) to throw an error / give an exception if the argument is invalid, but if persist:true saves the selection state, probably the error from set("path", ...) should be squelched.

So, I agree with you, and will implement your suggestion.

comment:2 Changed 7 years ago by bill

Resolution: fixed
Status: assignedclosed

In [30746]:

Set Tree::persist default to false, like other widgets, and also don't persist state of selected nodes. If desired, selectedPaths can be persisted by app code same as any other widget attribute. Fixes #14443, #16785, refs #14058 !strict.

comment:3 Changed 7 years ago by Peter Jekel

Bill,

To answer your question, yes both apps use the same cookie because the cookie name is derived from the widget id and does not include the URL.

If you are going to change the default behavour of a Tree widget property (persist) I would recommend to make a note of it in the release notes if you haven't done so already.

Thanks.

Last edited 7 years ago by Peter Jekel (previous) (diff)

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

Replying to peterj:

To answer your question, yes both apps use the same cookie because the cookie name is derived from the widget id and does not include the URL.

Yes, but don't the the pages have a different path, thus the scope of a cookie on one page doesn't carry over to the other page?

If you are going to change the default behavour of a Tree widget property (persist) I would recommend to make a note of it in the release notes if you haven't done so already.

Yes, will do.

comment:5 Changed 6 years ago by Peter Jekel

Bill,

Not sure if you changed your mind about the default for Tree::persist but in release 1.9.0b1 the default is still 'true'. If you did change your mind you need to update the current release notes.

comment:6 Changed 6 years ago by bill

Didn't change my mind, I thought I set it to false in [30746] (like the checkin comment says), but apparently I didn't. I'll fix that, thanks.

comment:7 Changed 6 years ago by bill

In [31065]:

meant to set persist:false (as a default) in [30746], refs #16785 !strict

Note: See TracTickets for help on using tickets.