|
Ошибка при добавлении обработчика к событию
Прочитал вот эту статью: http://javascript.ru/tutorial/events/crossbrowser
И либо что-то не правильно понял либо нашел ошибку в коде. Попробую описать, как происходит. Сначала добавляем к элементу обработчик (mousedown). В Event.add срабатывает такой код: if (!elem.events[type]) { elem.events[type] = {} elem.handle = function(event) { commonHandle(elem, type, event) } Теперь на элементе есть свойство handle, содержащее функцию. Далее добавляем обработчик к событию другого типа (mouseup). Т. к. тип другой, то проверка !elem.events[type] снова сработает и функция, содержащаяся в elem.handle , затрется новой функцией. Теперь если удалять второй добавленный обработчик то в Event.remove сработает elem.removeEventListener(type, elem.handle, false) , где пара type/handle та же что и была при addEventListener, но если удалять первый добавленный обработчик, то пара будет неправильная. Т. е. первому типу (mousedown) будет соответствовать вторая добавленная в elem.handle функция, а должна быть первая. Но оно затерлась. Попробую написать пример, в котором ошибка показывает себя. // получаем элемент var div1 = document.getElementById('div1'); // три обработчика var h1 = function(e) {div1.innerHTML += '1<br />'}; var h2 = function(e) {div1.innerHTML += '2<br />'}; var h3 = function(e) {div1.innerHTML += '3<br />'}; // добавляем обработчик к событию первого типа Event.add(div1, 'mousedown', h1); // к событию второго типа Event.add(div1, 'mouseup', h2); // удаляем первый добавленный обработчик // сейчас на mousedown ничего нет Event.remove(div1, 'mousedown', h1); // добавляем h3 к mousedown Event.add(div1, 'mousedown', h3); // h3 теперь должен срабатывать 1 раз т. к. на mousedown ничего не было // да и вообще h3 добавляли только 1 раз // но срабатывает 2 раза Полный пример: <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"> <html xmlns="http://www.w3.org/1999/xhtml"> <head> <title>blank</title> <script type="text/javascript"> Event = (function() { var guid = 0 function fixEvent(event) { if ( event.isFixed ) { return event } event.isFixed = true event.preventDefault = event.preventDefault || function(){this.returnValue = false} event.stopPropagation = event.stopPropagaton || function(){this.cancelBubble = true} if (!event.target) { event.target = event.srcElement } if (!event.relatedTarget && event.fromElement) { event.relatedTarget = event.fromElement == event.target ? event.toElement : event.fromElement; } if ( event.pageX == null && event.clientX != null ) { var html = document.documentElement, body = document.body; event.pageX = event.clientX + (html && html.scrollLeft || body && body.scrollLeft || 0) - (html.clientLeft || 0); event.pageY = event.clientY + (html && html.scrollTop || body && body.scrollTop || 0) - (html.clientTop || 0); } if ( !event.which && event.button ) { event.which = (event.button & 1 ? 1 : ( event.button & 2 ? 3 : ( event.button & 4 ? 2 : 0 ) )); } } function commonHandle(elem, type, event) { fixEvent(event) handlers = elem.events[type] for ( var g in handlers ) { var handler = handlers[g] var ret = handler.call(elem, event) if ( ret === false ) { event.preventDefault() event.stopPropagation() } } } return { add: function(elem, type, handler) { if (elem.setInterval && ( elem != window && !elem.frameElement ) ) { elem = window; } if (!handler.guid) { handler.guid = ++guid } if (!elem.events) { elem.events = {} } if (!elem.events[type]) { elem.events[type] = {} elem.handle = function(event) { commonHandle(elem, type, event) } if (elem.addEventListener) elem.addEventListener(type, elem.handle, false) else if (elem.attachEvent) elem.attachEvent('on' + type, elem.handle) } elem.events[type][handler.guid] = handler }, remove: function(elem, type, handler) { var handlers = elem.events && elem.events[type] if (!handlers) return delete handlers[handler.guid] for(var any in handlers) { break; } if (!any) { if (elem.removeEventListener) elem.removeEventListener(type, elem.handle, false); else if (elem.detachEvent) elem.detachEvent('on' + type, elem.handle); delete elem.events[type] } } } }()) window.onload = function() { var div1 = document.getElementById('div1'); var h1 = function(e) {div1.innerHTML += '1<br />'}; var h2 = function(e) {div1.innerHTML += '2<br />'}; var h3 = function(e) {div1.innerHTML += '3<br />'}; Event.add(div1, 'mousedown', h1); Event.add(div1, 'mouseup', h2); Event.remove(div1, 'mousedown', h1); Event.add(div1, 'mousedown', h3); }; </script> </head> <body> <div id="div1" style="width: 200px; height: 600px; background: #cfc;"></div> </body> </html> |
Я бы не стал беспокоиться, если бы не это:
Цитата:
В любом случае получается, что на авторитетном сайте предлагается код с ошибками. |
Большое спасибо за столь хорошо оформленный багрепорт, ув. Riim!
В статье действительна была ошибка. Ввиду того, что статья важная - исправил ее тут же.. Сейчас 1 час ночи, надеюсь, не добавил новых ошибок. Если что - пишите. Сжатую версию также обновил. |
Возможно, стоит еще кое-что подправить:
1. в commonHandle переменная handlers попадает в window. Нужно var добавить. 2. непонятно зачем в elem.handle проверка (typeof Event !== "undefined") Возможно, на тот случай, когда обработчик будет добавляться к элементу из другого фрейма (или сам Event в другом фрейме, а элемент в текущем). Но и в этом случае Event должен быть доступен. 3. содержимое commonHandle вынесено в отдельную функцию. Вероятно для увеличения производительности Event.add, но это делается в ущерб производительности elem.handle, которая имеет заметно больший приоритет. 4. конструкции типа: for (var any in elem.events ) break; if ( !any ) { delete elem.handle delete elem.events } возможно стоит заменить на: for (var any in elem.events ) return; delete elem.handle delete elem.events Там еще множество разных оптимизаций можно применить, но все они в ущерб читабельности. А эта вроде нет. |
Цитата:
Цитата:
Цитата:
Цитата:
|
Цитата:
1. все конструкции типа: a = a || b; полезно заменить на: a || (a = b); Это неплохо увеличивает производительность. Вот так, например, выглядит мой вариант Event.fix : if (e._fixed) return e; e._fixed = true; e.target || (e.target = e.srcElement || $d); e.relatedTarget || e.fromElement && (e.relatedTarget = e.fromElement == e.target ? e.toElement : e.fromElement); if (e.pageX == null && e.clientX != null) { var docElem = $d.documentElement, body = $d.body || {}; e.pageX = e.clientX + (window.pageXOffset || docElem.scrollLeft || body.scrollLeft || 0) - (docElem.clientLeft || 0); e.pageY = e.clientY + (window.pageYOffset || docElem.scrollTop || body.scrollTop || 0) - (docElem.clientTop || 0); } e.which || e.button && (e.which = e.button & 1 ? 1 : e.button & 2 ? 3 : e.button & 4 ? 2 : 0); e.preventDefault || (e.preventDefault = preventDefault); e.stopPropagation || (e.stopPropagation = stopPropagation); return e; preventDefault и stopPropagation создаются заранее один раз. 2. в commonHandle переменные handler и ret используются один раз. Лучше вычислять их прямо там, где они используются: function commonHandle(event) { event = fixEvent(event) var handlers = this.events[event.type] for ( var g in handlers ) { if ( handlers[g].call(this, event) === false ) { event.preventDefault() event.stopPropagation() } } } 3. куски if (!handler.guid) handler.guid = ++guid и elem.events[type][handler.guid] = handler полезно объединить в одну конструкцию: elem.events[type][handler.guid || (handler.guid = ++guid)] = handler; Это позволяет избежать лишнего обращения к свойству объекта (.guid) и минус одна операция булевого отрицания (или как она там называется). 4. fixEvent можно сделать публичным. Во-первых, он может понадобиться при обработке в атрибуте элемента, куда код добавляется не через Event.add, во-вторых, может возникнуть необходимость переопределить его (менее вероятно) или обернуть в другую функцию (вот это очень вероятно) для увеличения функциональности. -------- Не надоел еще? |
Цитата:
|
Цитата:
|
Это не смысл замены, а логика замены. Смысла в этом нет.
|
Если вы не видите в этом смысла, то это не значит, что его нет.
|
Часовой пояс GMT +3, время: 04:44. |
|