Questions: 1) LzInputTextSprite#492, I don't see the point of this guarding `if`, since it appears to be guarding individual tests that simply repeat it?
2) LzInputTextSprite#502 says "this is Firefox specific code". Shouldn't it be in a quirk then? Maybe the comment is just mis-placed? 3) LzInputTextSprite#551 seems like a redundant `if`? Ditto at 556? 4) LzInputTextSprite#708 `LzSprite.prototype.setClickable.apply(this, [true]);` could be changed to `call` and not cost the array, or use your fake super-call technique. Of course, it really should be calling LzTextSprite.prototype, since that is its superclass, right? 5) LzInputTextSprite#1101 Yow! 6) LzMouseKernel#28, LzSprite#212 I'm concerned using . instead of [' could lead to warnings in debug mode. 7) LzMouseKernel#137, this technique will call the method with an empty context, is that going to work? attach/removeEventHandler do not use `this`? If so, perhaps add a comment? 8) LzMouseKernel#1531 Do we really still support FF 1.5? 9) LzMouseKernel#1691, LzMouseKernel#1715 Maybe `return false`, just to document that this is a function with a boolean result? 10) General question about LzMouseKernel.__clickDispatcher: Does it still make sense to talk about bubbling here? I thought this handler is now on the root sprite div, so where is the event going to bubble to? Otherwise technically approved, to the best of my ability. This is tricky stuff! Hope the QA reviewer will be thorough! On 2010-09-21, at 16:51, Max Carlson wrote: > Change 20100921-maxcarlson-L by maxcarl...@friendly on 2010-09-21 13:14:17 PDT > in /Users/maxcarlson/openlaszlo/trunk2 > for http://svn.openlaszlo.org/openlaszlo/trunk > > Summary: Simplify mouse events > > Bugs Fixed: LPP-8470 - Simplify IE 7-specific mousehandling logic > > Technical Reviewer: ptw > QA Reviewer: hminsky > > Overview: This change uses a single (canvas) div for handling mouse events, > instead of registering on all click divs. A lot of simplification falls out > of this. > > Details: LzSprite - Use LzSprite.__setClickable() to register for all mouse > events at root sprite init time. Move quirks.prevent_selection clause from > global scope of LzInputTextSprite. Remove __setClickable() calls, since > individual clickdivs are no longer made clickable. Unify (almost) all mouse > event handling logic in LzSprite.__clickDispatcher(), simplify > __mouseEvent(). Improve __findParents() to return the first item found, use > in __isMouseOver(). __globalmouseup() is simplified. Clean up > setWidth/Height() and destroy() logic. Test for quirks.fix_clickable in > setCursor(). > > LzTextSprite - Update setSelectable() to use explicit handler for allowing > selection, to prevent event bubbling. Remove __setClickable() calls, since > individual clickdivs are no longer made clickable. Override __mouseEvent() > to return based on the value of selectable, to allow selection to happen. > > LzMouseKernel - Eliminate __mouseupEvent() since global mosue up is handled > by LzSprite.__clickDispatcher(). Explicitly return true for real onmousemove > events to keep text selection working. > > LzInputTextSprite - Wrap this.dragging property initialization in > quirks.autoscroll_textarea to show where it's used. Inputtexts are always > clickable, so call setClickable(true). Override setClickable() to cache the > value locally and ensure the inputtext remains clickable. Remove unused > __handlemouse() method. Use explicit handler for allowing selection, to > prevent event bubbling. Minimize events registered by __setTextEvents(), and > forward to __mouseEvent() when needed. Fix braino in setTextColor() > > Tests: All apps run as before in DHTML on Firefox, Safari and IE. > test/lfc/legals/keyboardandmouse.lzx?lzr=dhtml&lzt=html comes much closer to > matching swf. > > Files: > M WEB-INF/lps/lfc/kernel/dhtml/LzSprite.js > M WEB-INF/lps/lfc/kernel/dhtml/LzTextSprite.js > M WEB-INF/lps/lfc/kernel/dhtml/LzMouseKernel.js > M WEB-INF/lps/lfc/kernel/dhtml/LzInputTextSprite.js > > Changeset: > http://svn.openlaszlo.org/openlaszlo/patches/20100921-maxcarlson-L.tar
