|
Ошибка при добавлении обработчика к событию
Прочитал вот эту статью: 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, время: 21:03. |
|