Javascript.RU

Создать новую тему Ответ
 
Опции темы Искать в теме
  #1 (permalink)  
Старый 27.04.2009, 09:50
Аватар для Riim
Рассеянный профессор
Отправить личное сообщение для Riim Посмотреть профиль Найти все сообщения от Riim
 
Регистрация: 06.04.2009
Сообщений: 2,379

Ошибка при добавлении обработчика к событию
Прочитал вот эту статью: 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>
Ответить с цитированием
  #2 (permalink)  
Старый 27.04.2009, 10:11
Аватар для Riim
Рассеянный профессор
Отправить личное сообщение для Riim Посмотреть профиль Найти все сообщения от Riim
 
Регистрация: 06.04.2009
Сообщений: 2,379

Я бы не стал беспокоиться, если бы не это:
Цитата:

Большой вклад в это внесли Dead Edwards и Tino Zeidel, на библиотеках которых зачастую и основан современный код. Например, это так для dojo toolkit и jQuery.
Попробовал в Jquery. Вроде все нормально.

В любом случае получается, что на авторитетном сайте предлагается код с ошибками.
Ответить с цитированием
  #3 (permalink)  
Старый 02.05.2009, 01:04
Аватар для Илья Кантор
Администратор
Отправить личное сообщение для Илья Кантор Посмотреть профиль Найти все сообщения от Илья Кантор
 
Регистрация: 25.05.2007
Сообщений: 1,221

Большое спасибо за столь хорошо оформленный багрепорт, ув. Riim!

В статье действительна была ошибка.
Ввиду того, что статья важная - исправил ее тут же..
Сейчас 1 час ночи, надеюсь, не добавил новых ошибок. Если что - пишите.

Сжатую версию также обновил.
Ответить с цитированием
  #4 (permalink)  
Старый 02.05.2009, 07:16
Аватар для Riim
Рассеянный профессор
Отправить личное сообщение для Riim Посмотреть профиль Найти все сообщения от Riim
 
Регистрация: 06.04.2009
Сообщений: 2,379

Возможно, стоит еще кое-что подправить:

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


Там еще множество разных оптимизаций можно применить, но все они в ущерб читабельности. А эта вроде нет.
Ответить с цитированием
  #5 (permalink)  
Старый 02.05.2009, 11:51
Аватар для Илья Кантор
Администратор
Отправить личное сообщение для Илья Кантор Посмотреть профиль Найти все сообщения от Илья Кантор
 
Регистрация: 25.05.2007
Сообщений: 1,221

Сообщение от Riim Посмотреть сообщение
1. в commonHandle переменная handlers попадает в window. Нужно var добавить.
Ага, почистил.

Цитата:
2. непонятно зачем в elem.handle проверка (typeof Event !== "undefined")
В статье про это сказано - это для фильтрации редкой ошибки с unload страницы.


Цитата:
3. содержимое commonHandle вынесено в отдельную функцию. Вероятно для увеличения производительности Event.add, но это делается в ущерб производительности elem.handle, которая имеет заметно больший приоритет.
Это чтобы не создавать кучу лишних функций commonHandle и не расходовать тем самым память. Конечно, можно ее заинлайнить.

Цитата:
4. конструкции типа:
...
Там еще множество разных оптимизаций можно применить, но все они в ущерб читабельности. А эта вроде нет.
Да, приятное исправление, сделал его. Особо серьезных оптимизаций там даже в ущерб читабельности не сделаешь имхо.
Ответить с цитированием
  #6 (permalink)  
Старый 02.05.2009, 13:10
Аватар для Riim
Рассеянный профессор
Отправить личное сообщение для Riim Посмотреть профиль Найти все сообщения от Riim
 
Регистрация: 06.04.2009
Сообщений: 2,379

Сообщение от Илья Кантор
Особо серьезных оптимизаций там даже в ущерб читабельности не сделаешь имхо.
Ну тогда продолжим :

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, во-вторых, может возникнуть необходимость переопределить его (менее вероятно) или обернуть в другую функцию (вот это очень вероятно) для увеличения функциональности.

--------

Не надоел еще?

Последний раз редактировалось Riim, 04.05.2009 в 12:58.
Ответить с цитированием
  #7 (permalink)  
Старый 02.05.2009, 14:43
Новичок на форуме
Отправить личное сообщение для Kolyaj Посмотреть профиль Найти все сообщения от Kolyaj
 
Регистрация: 19.02.2008
Сообщений: 9,177

Сообщение от Riim
все конструкции типа:
a = a || b;
полезно заменить на:
a || (a = b);
Жуть какая.
Ответить с цитированием
  #8 (permalink)  
Старый 02.05.2009, 15:15
Аватар для Riim
Рассеянный профессор
Отправить личное сообщение для Riim Посмотреть профиль Найти все сообщения от Riim
 
Регистрация: 06.04.2009
Сообщений: 2,379

Сообщение от Kolyaj
Жуть какая.
Смысл замены в том что "а" не будет лишний раз перезаписываться, когда он/она уже имеет нужное значение.
Ответить с цитированием
  #9 (permalink)  
Старый 02.05.2009, 15:16
Новичок на форуме
Отправить личное сообщение для Kolyaj Посмотреть профиль Найти все сообщения от Kolyaj
 
Регистрация: 19.02.2008
Сообщений: 9,177

Это не смысл замены, а логика замены. Смысла в этом нет.
Ответить с цитированием
  #10 (permalink)  
Старый 02.05.2009, 15:27
Аватар для Riim
Рассеянный профессор
Отправить личное сообщение для Riim Посмотреть профиль Найти все сообщения от Riim
 
Регистрация: 06.04.2009
Сообщений: 2,379

Если вы не видите в этом смысла, то это не значит, что его нет.

Последний раз редактировалось Riim, 02.05.2009 в 15:41.
Ответить с цитированием
Ответ


Опции темы Искать в теме
Искать в теме:

Расширенный поиск


Похожие темы
Тема Автор Раздел Ответов Последнее сообщение
как при добавлении изображения на сервер считывать имя в БД? solomusic Серверные языки и технологии 3 12.06.2008 23:28