cytoscape.js-expand-collapse icon indicating copy to clipboard operation
cytoscape.js-expand-collapse copied to clipboard

Exception - Expanding Collapsed Edges of a Collapsed Child

Open sashokbg opened this issue 2 years ago • 9 comments

Hello, I have stumbled upon two bugs (possible more) when I try to expand edges that are pointing to a child of a collapsed node:

I have created a small demo file to reproduce the issue - you can check out at https://github.com/sashokbg/cytoscape.js-expand-collapse.git branch bug/child_collapse_edges file: demo/demo-bug.html

Bug 1 Disappearing edges - Steps to reproduce:

  1. Starting graph image

  2. Click on "collapse edges" button

  3. Close node 1

  4. Close node a

  5. Click on expand all edges <-- exception

  6. Edges disappear

Stack:

Uncaught Error: Can not create edge `e1` with nonexistant source `1`
    Ee cytoscape.min.js:23
    restore cytoscape.min.js:29
    add cytoscape.min.js:29
    expandEdge expandCollapseUtilities.js:786
    expandEdges index.js:272
    forEach cytoscape.min.js:29
    expandEdges index.js:271
    expandAllEdges index.js:360
    <anonymous> demo-bug.html:151
    EventListener.handleEvent* demo-bug.html:150
    EventListener.handleEvent* demo-bug.html:16

Bug 2 - Expanded edges change source / destination

  1. Starting graph image

  2. Click on "collapse all"

  3. Click on "collapse edges"

  4. Click on "expand all" <-- bug Edges are not having as a source the parent node "a" instead of "1" image

  5. Click on "expand edges" <-- bug Bug persist after expanding edges, now original links are wrongly having node "a" as a source image

Observations

If we first collapse all nodes and then we expand them it seems that the exception issue is gone. I have however still managed to reproduce the second bug in some scenarios when manually expanding and collapsing the child nodes and edges.

Any help or ideas are greatly appreciated ! Have a nice day! Alex

sashokbg avatar Nov 06 '21 14:11 sashokbg

I have debugged for a while and it seems both problems are coming from manipulating collapsed edges.

First issue The function barrowEdgesOfcollapsedChildren.

In the case of the first bug, it turns out when collapsing a node, and it has edges that have already been collapsed, we are not properly creating the _cy-expand-collapse-collapsed-edge_s.

A quick fix for this issue is to just expand collapsed edges when collapsing a node. This way the meta edges are properly calculated.

Second issue Same solution can be applied in the repairEdges function:

    node.connectedEdges('.cy-expand-collapse-collapsed-edge').forEach((edge) => this.expandEdge(edge));

sashokbg avatar Nov 06 '21 16:11 sashokbg

A better fix is available at #131

sashokbg avatar Nov 08 '21 14:11 sashokbg

Can anyone of the maintainers please take a look ? @metincansiper @hasanbalci

sashokbg avatar Nov 22 '21 23:11 sashokbg

Anyone ?

sashokbg avatar Jan 12 '22 10:01 sashokbg

I came here to report this exact issue. Can we get any insight from the maintainers to respond or accept #131? Or alternatively explain why they cannot/will not use this fix?

jonlev1n avatar May 09 '22 16:05 jonlev1n

I came here to report this exact issue. Can we get any insight from the maintainers to respond or accept #131? Or alternatively explain why they cannot/will not use this fix?

+1

Revadike avatar May 16 '22 03:05 Revadike

The first bug doesn't seem like a bug to me. It is a misuse of the API. You should not let the user expand a cy-expand-collapse-meta-edge It is a meta edge created after you collapsed a compound node. First, the compound should be expanded since the source/target doesn't exist.

For the second, I can also say this is not a bug if you first expand the collapsed edge. But this time, expanding all the collapsed edges connected to the compounds before it is going to be expanded might be a good use case.

canbax avatar May 16 '22 06:05 canbax

"Collapse/expand edges" feature was added later. It seems that its interaction with the "collapse/expand nodes" feature wasn't considered comprehensively during addition of this new feature. Therefore, these side-effects/bugs are possible. However, expand-collapse is a rather complex extension and solving such bugs requires a detailed analysis and effort. We don't want to add bug-specific patches since they may cause other unexpected issues.

Unfortunately, we currently don't have enough sources to solve this issue. My suggestion for you to avoid these cases is either to use @sashokbg's branch that possibly has a fix (I didn't try it) or to find some workarounds while presenting these features to your users.

hasanbalci avatar May 16 '22 07:05 hasanbalci

The first bug doesn't seem like a bug to me. It is a misuse of the API. You should not let the user expand a cy-expand-collapse-meta-edge It is a meta edge created after you collapsed a compound node. First, the compound should be expanded since the source/target doesn't exist.

For the second, I can also say this is not a bug if you first expand the collapsed edge. But this time, expanding all the collapsed edges connected to the compounds before it is going to be expanded might be a good use case.

I am sorry to disagree but those are obviously bugs. Calling an API function that completely breaks the library, or wrongly reconstructs your edges afterwards is a definition of a bug. Either the API should not allow such scenarios or if does they should not break anything.

sashokbg avatar May 16 '22 20:05 sashokbg