Skip to content
This repository was archived by the owner on Mar 14, 2020. It is now read-only.

[ADM] Fix bug: ID auto numbering occurring in reverse.#243

Closed
xuqingkuang wants to merge 1 commit into
intel:masterfrom
xuqingkuang:fixIDReverse
Closed

[ADM] Fix bug: ID auto numbering occurring in reverse.#243
xuqingkuang wants to merge 1 commit into
intel:masterfrom
xuqingkuang:fixIDReverse

Conversation

@xuqingkuang

Copy link
Copy Markdown
Contributor

No description provided.

@zhizhangchen

Copy link
Copy Markdown
Contributor

Why is the problem related to findNodesByProperty?

@xuqingkuang

Copy link
Copy Markdown
Contributor Author

@zhizhangchen Set a break point at ADMNode.prototype.setProperty() and set the condition to 'property == "id"', then drop a checkbox group into canvas you will see:

Update the checkbox group ID is trigger by ModelUpdated event, it catches by projectView's _modelUpdatedHandler(), then it use findNodesByProperty() to traverse the children checkboxes, and use generateUniqueProperty() to generate ID.

The problem is generateUniqueProperty() generate correct IDs - checkbox1, checkbox2 and checkbox3, but findNodesByProperty() returns the nodes reversed - node3, node2, node1.

So it applied incorrect IDs - node3 applied 'checkbox1', node2 applied 'checkbox2' and node1 applied 'checkbox3'.

In fact, I have no idea why here traverse the node reversed, traverse in order maybe work too.

BTW: The function only called in project.js so far.

$ grep -nr findNodesByProperty *
src/js/adm.js:1475:ADMNode.prototype.findNodesByProperty = function (propertyFilter) {
src/js/adm.js:1491: result = result.concat(children[i].findNodesByProperty(propertyFilter));
src/js/projects.js:1023: matched = node.findNodesByProperty($.rib.pmUtils.relativeFilter);

This was referenced Aug 27, 2012
@grgustaf

Copy link
Copy Markdown
Contributor

(Because we didn't address the real problem, this bug could resurface if something changes that gets the ID properties accessed before we get to this function.)

@grgustaf

Copy link
Copy Markdown
Contributor

Merged.

@grgustaf grgustaf closed this Aug 29, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants