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: UPDATED: 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: Updated to address Tucker's comments:
> 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?
It's there to prevent the mouse events from being forwarded, see the return at
the end of the block. I improved the comment.
> 2) LzInputTextSprite#502 says "this is Firefox specific code". Shouldn't it
> be in a quirk then? Maybe the comment is just mis-placed?
Improved the comment.
> 3) LzInputTextSprite#551 seems like a redundant `if`? Ditto at 556?
Fixed.
> 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?
Fixed.
> 5) LzInputTextSprite#1101 Yow!
Indeed. Whoopsie!
> 6) LzMouseKernel#28, LzSprite#212 I'm concerned using . instead of [' could
> lead to warnings in debug mode.
I disagree. Either way is valid. Also, I don't think we do that kind of
testing in the generated LFC, for performance reasons... And we only really
warn for null properties for swf8 user code, right?
> 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?
I added a comment.
> 8) LzMouseKernel#1531 Do we really still support FF 1.5?
Probably not, but I was worried that removing this would have side effects in
other browsers. I added a TODO to remember to look later.
> 9) LzMouseKernel#1691, LzMouseKernel#1715 Maybe `return false`, just to
> document that this is a function with a boolean result?
Done.
> 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?
I think it does make sense. I added a comment.
> Otherwise technically approved, to the best of my ability. This is tricky
> stuff!
Otherwise the same:
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
