Permalink
Browse files

Remove JX.DOM.listen reliance on DOM ID

Summary:
Currently,

  <div id="foo">...</div>

  // foo is a reference to the DOM node
  JX.DOM.listen(foo, 'click', [], listener);

  foo.id = null; // makes listener not work

JX.DOM.listen relies on the DOM ID, which leads to issues when you
change the DOM node ID or have another DOM node with the same ID
(possible if you swap nodes)

Remove reliance on DOM ID and instead set another hidden attribute
(data-autoid) which is used in Stratcom

Test Plan:
  <div id="foo">foo</div>

  var foo = JX.$('foo');
  JX.DOM.listen(foo, 'click', [], function() { debugger; });

  // click and see listener is fired
  foo.id = null;

  // click and see listener is still fired. not fired before diff

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3704
  • Loading branch information...
1 parent adfcfcf commit d3fb352b0f35813ec96c1d3e396051e0db672a1c @paulshen paulshen committed Oct 15, 2012
Showing with 20 additions and 7 deletions.
  1. +5 −0 src/core/Stratcom.js
  2. +15 −7 src/lib/DOM.js
View
@@ -315,6 +315,11 @@ JX.install('Stratcom', {
}
}
+ var auto_id = cursor.getAttribute('data-autoid');
+ if (auto_id) {
+ push('autoid:' + auto_id, cursor, distance);
+ }
+
++distance;
cursor = cursor.parentNode;
}
View
@@ -311,6 +311,7 @@ JX.$N = function(tag, attr, content) {
JX.install('DOM', {
statics : {
_autoid : 0,
+ _uniqid : 0,
_metrics : {},
@@ -639,21 +640,21 @@ JX.install('DOM', {
}
}
- var id = ['id:' + JX.DOM.uniqID(node)];
+ var auto_id = ['autoid:' + JX.DOM._getAutoID(node)];
path = JX.$AX(path || []);
if (!path.length) {
- path = id;
+ path = auto_id;
} else {
for (var ii = 0; ii < path.length; ii++) {
- path[ii] = id.concat(JX.$AX(path[ii]));
+ path[ii] = auto_id.concat(JX.$AX(path[ii]));
}
}
return JX.Stratcom.listen(type, path, callback);
},
uniqID : function(node) {
if (!node.getAttribute('id')) {
- node.setAttribute('id', 'autoid_'+(++JX.DOM._autoid));
+ node.setAttribute('id', 'uniqid_'+(++JX.DOM._uniqid));
}
return node.getAttribute('id');
},
@@ -850,9 +851,16 @@ JX.install('DOM', {
* @param Node Node to move document scroll position to, if possible.
* @return void
*/
- scrollTo : function(node) {
- window.scrollTo(0, JX.$V(node).y);
- }
+ scrollTo : function(node) {
+ window.scrollTo(0, JX.$V(node).y);
+ },
+
+ _getAutoID : function(node) {
+ if (!node.getAttribute('data-autoid')) {
+ node.setAttribute('data-autoid', 'autoid_'+(++JX.DOM._autoid));
+ }
+ return node.getAttribute('data-autoid');
+ }
}
});

0 comments on commit d3fb352

Please sign in to comment.