Community
Participate
Working Groups
1. Install this outliner: http://mamacdon.github.com/outliner/outlinerPlugin.html 2. Open this file from the orion source code: bundles/org.eclipse.orion.client.core/web/orion/sites/sitesExplorer.js 3. I see a bunch of outline items with duplicated IDs: > <tr id="1347648703464" class="navRow treeTableRow" style="vertical-align: baseline; "> > <span style="padding-left: 16px; "><span id="1347648703464__expand" class="modelDecorationSprite core-sprite-closedarrow"></span><a style="cursor: pointer; " class="navlinkonpage">function()</a></span></tr> > <tr id="1347648703464" class="navRow treeTableRow" style="vertical-align: baseline; "> > <span style="padding-left: 16px; "><span id="1347648703464__expand" class="modelDecorationSprite core-sprite-closedarrow"></span><a style="cursor: pointer; " class="navlinkonpage">function()</a></span></tr> > ... This causes funky behavior when navigating through the outline tree.
I actually wrote a bunch of code to prevent this from happening, and tested it for some files, but there must be a corner case I didn't check.
It's a timing issue of some kind. If I step through with the debugger, I can see all the duplicate id checking happening and cursoring works as expected. Investigating...
Fixed in http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=53f60294d646073149c98180acc5fb6727c49f48 This was the "using 'for var in array' bug" http://stackoverflow.com/questions/1885317/strange-behavior-in-javascript-enhanced-for-in-loop/1885365#1885365 The children were being iterated using "for var in children" instead of iterated numerically. I'm still not sure why this caused the bug, because I wouldn't expect any of the id generation code to rely on order, but perhaps we bombing out of an id computation while iterating other properties. I fixed it on a hunch and it worked. There are a couple other fixes in the commit, things I cleaned up while trying to figure out the problem.
I noticed that the duped ids always seemed to be timestamps. Perhaps if the resolution of Date.getTime() is not very precise, it's possible for several model items to have IDs generated with the same getTime() value?
I believe I am seeing the issue described in comment 4 with Orion 2.0, where duplicate id's are being replaced with the same timestamp.
Reopening -- this problem still exists. To summarize, I think the issue here is with the algorithm in OutlineModel.prototype.getId(). That function is called on each item in the outline model to produce a unique ID. It first tries to use the item's label, and if the label is not unique, falls back to `new Date().getTime()`. If several items in the model have the same label, and iteration over the outline model happens faster than observable changes in getTime(), then several calls to getId() will return the same timestamp for different items. That gives us ID collisions, which cause the UI to behave badly (eg. clicking one node in the outliner causes a different node to expand). The outline model is just a tree. We can uniquely number its items by traversing the tree and incrementing a counter. Then we use the node number as an item's ID. No chance of collisions (at least not with a single outline renderer on the page, which is all we care about right now). The traversal only has to happen once, we can cache the IDs from then on.
Also: the duplicate IDs are what causes the accessibility features (highlighting a whole row, keyboard navigation, etc) to fail in the outline view.
One solution that I have tried is to add a counter to the OutlineModel constructor, and then increment the counter when a duplicate is found: function OutlineModel(items, rootId) { this.items = items; this.root = {children: items}; this.root.outlinerId = rootId; this.idItemMap = {}; this._nextID = 0; } OutlineModel.prototype.getId = function(/* item */ item){ .... if ((this.idItemMap[id] && this.idItemMap[id]!== item) || lib.node(id)) {// see https://bugs.eclipse.org/bugs/show_bug.cgi?id=389760 id = this._nextID.toString(); //new Date().getTime().toString(); this._nextID++; this.idItemMap[id] = item; item.outlinerId = id; } else { this.idItemMap[id] = item; } ....
I ran into this bug while working on another feature. Here's my proposed fix: https://github.com/elijahe/orion.client/commit/8ccf4fa0300c12708f3ae51c57656c81cff10730 Please review and commit if appropriate.
Merged: http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=bf9abe3
