Hey Max, Can you recall why the "deselect()" call was added in the mouse-out handling in LzInputTextSprite below? I'm trying to fix this bug (LPP-9269) and wanted to know if I can just remove that call.
On Wed, Jan 26, 2011 at 8:50 PM, Henry Minsky <[email protected]>wrote: > I'm looking at LPP-9629 , which is a bug that shows up in DHTML in > Firefox and IE, where when you select some > inputtext by dragging and release the mouseup outside of the view, the > selection and focus gets confused. > > I found that if I commented out the call to 'sprite.deselect()' in > LzInputText.__textEvent, that the bug goes away. > I'm trying to figure out why we have that call at all, it seems like the > desired behavior is that the text selection > remain regardless of where you mouse up... > > LzInputTextSprite.prototype.__textEvent = function ( evt ){ > > } else if (eventname === 'onmouseup') { > evt.cancelBubble = true; > // if the mouse isn't over us, send an onmouseupoutside evemt > if (! sprite.__isMouseOver()) { > sprite.__globalmouseup(evt); > // deselect ourselves > // sprite.deselect(); > ^^^^^^ > } else { > sprite.__mouseEvent(eventname); > } > } > > The deselect() call was added in this change r17585 | max | 2010-09-23 16:32:42 -0400 (Thu, 23 Sep 2010) | 71 lines Changed paths: M /openlaszlo/trunk/WEB-INF/lps/lfc/kernel/dhtml/LzInputTextSprite.js M /openlaszlo/trunk/WEB-INF/lps/lfc/kernel/dhtml/LzMouseKernel.js M /openlaszlo/trunk/WEB-INF/lps/lfc/kernel/dhtml/LzSprite.js M /openlaszlo/trunk/WEB-INF/lps/lfc/kernel/dhtml/LzTextSprite.js Change 20100921-maxcarlson-L by maxcarlson@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. > > -- > Henry Minsky > Software Architect > [email protected] > > > -- Henry Minsky Software Architect [email protected]
