I'm going to suggest that since this change was supposedly in support of IE7 and IE7 is no longer supported, you should feel free to remove the offending clause.
It is certainly the case that in every other UI I can find, mouseup outside of a text box where you started a selection leaves the text selected. This allows you to be sloppy when selecting everything before/after where you mousedown-ed -- you can just sweep out of the text box. It's horrible UI to make you have to stay _in_ the text box to select to the beginning or end. Unless Max can prove this deselect is needed for some even more horrible bug, lets remove this and move on. On 2011-02-01, at 09:07, Henry Minsky wrote: > 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]
