approved On Wed, Apr 14, 2010 at 6:38 PM, Max Carlson <[email protected]> wrote:
> Change 20100410-hqm-e by [email protected] on 2010-04-10 19:31:18 EDT > in /Users/hqm/openlaszlo/trunk > for http://svn.openlaszlo.org/openlaszlo/trunk > > Summary: UPDATED ^2 BY MAX: make autosizing inputtext recompute it's height > properly when user types > > New Features: > > Bugs Fixed: LPP-8591 Something mysterious about scrollHeight property in > DHTML, LPP-7832 - Kernel API getTextHeight should be renamed getLineHeight > to avoid confusion > > Technical Reviewer: hminsky > QA Reviewer: ptw > Doc Reviewer: (pending) > > Documentation: > > Release Notes: > > Overview: Updated to address Tucker's comments: > > 1) In CSS, you don't need to specify units for `0`: `0px` is superfluous > (In LzSprite.js). So is `"0px"`, and `0+"px"`! (In LzIputTextSprite.js) > > I updated all 0px values to 0 and changed LzKernelUtils.CSSDimension() to > return 0 as-is. > > 2) I think a better idiom for (m == true) is (!!m). My 2p. > > Changed. > > 3) I guess you are saying that it is more efficient to store the tagname > as a sprite property than to ask for it back from the DOM? Perhaps a > comment to that effect so future generations do not wonder? > > I added a comment. > > Issues: > > 1) Why do we need this {setWidth,setHeight}/skipupdate flag? Is there > ever a time that the generic LFC calls setHeight that it would need a > callback from the kernel to actually install the height? It would seem > better to me to fix up the sprite implementation to not use the LFC->kernel > API internally when it is trying to send an update the other way (which is > what the old `force` flag was for). > > Yeah, you're right. I managed to eliminate or rationalize all internal > sprit e.setWidth/Height() calls, since _updateSize() was updating that with > definitive values anyway. > > I made sure LzText.getTextfieldHeight/TextWidth() notified the kernel to > bypass text initialization options. > > 2) I don't understand the change in LzTextSprite/__fontloaded from setting > the height to the height of the contents to setting it to just 1-line high. > Why? > > Good catch! That is what it was doing before because getTextHeight() -> > getLineHeight(). > > 3) I realize this is not part of your change, but I am concerned about the > TODO on line 529 ff of LzTextSprite. It seems we _have_ implemented using > the correct tag for the measurement node, but we _have_not_ made that tag > part of the cache key (the cache key is hardwired to 'div'). Isn't that > going to screw up measurement if we happen to have a <text> and <inputtext> > with the same content? Shouldn't the `'div'` on line 533 be > `this.scrolldivtagname` instead? > > Yes, this has been fixed. Perhaps we'd get better measurements if we used > actual <input or <textarea objects? > > 4) The comment at LaszloView#2440 is garbled. Same at LaszloView#2473 > > Fixed. > > Otherwise, the same: > > I tweaked this change a bit in order to make it more efficient. I also > renamed some args. Now test/lztest/lztest-textheight.lzx lists all expected > warnings, and things look more visually consistent across dhtml, swf8 and > swf10. And of course, variable-height inputtexts resize when you type in > them. And, I found and fixed a text measurement bug in IE where the first > time a string was measured, it could be inaccurate. > > + make autosizing inputtext recompute it's height properly when user types > > Details: > > WEB-INF/lps/lfc/kernel/dhtml/LzTextSprite.js: 0px -> 0, store > scrolldivtagname for faster lookups, remove unneeded calls to > setWidth/Height() since __updatefieldsize covers them, optimize style > measurement key lookups, move text measurement to LzFontManager, fix > setWidth() to account for text-indent per Andre's comments, skip unneeded > updates to scroll div in setHeight(), > > LzFontManager - Move text measurement from LzTextSprite > > WEB-INF/lps/lfc/kernel/swf/LzTextSprite.as: Rename getTextHeight -> > getLineHeight. Add new 'force' arg to getTextfieldHeight (unused in this > implementation, but for compatibiltiy > with DHTML) > > WEB-INF/lps/lfc/kernel/swf9/LzTextSprite.as: Rename getTextHeight -> > getLineHeight. Add new 'force' arg to getTextfieldHeight (unused in this > implementation, but for compatibiltiy with DHTML) > > WEB-INF/lps/lfc/kernel/dhtml/LzSprite.js: Added quirk for setting height to > zero in order to get proper scroll height in IE and Safari (see > __textEvent() in dhtml/LzInputTextSprite.js), 0px -> 0, default > lzswfinputtextmultiline CSS to whiteSpace: 'pre-wrap', use > LzFontManager.__createContainerDiv() to create font measurement container. > > WEB-INF/lps/lfc/kernel/dhtml/LzInputTextSprite.js:When a multiline field is > supposed to be autosize it's height, update the position for IE and Safari > quirk, when user types into the field. Track scrolldivtagname, rely on > view.inputtextevent to call __updatefieldsize(), 0px -> 0. > > LzKernelUtils: Return 0 immediately in CSSDimension() > > WEB-INF/lps/lfc/views/LzInputText.lzs: When 'onchange' event is received > from kernel, multiline field should update it's height to the measured text > height, using 'updateHeight' instead of setHeight (so it doesn't turn it > into a fixed height field) > > WEB-INF/lps/lfc/views/LzText.lzs: Autosizing text fields should call > updateHeight, not setHeight, when updating their height, so they don't turn > into fixed height fields. Call sprite APIs directly for text size > information, and pass flag for getTextHeight() so sprites know when the call > is from user code. > > WEB-INF/lps/lfc/views/LaszloView.lzs: updateHeight and updateWidth methods > update the memo cache vars > > WEB-INF/lps/server/src/org/openlaszlo/sc/CommonGenerator.java: fix for an > unrelated bug that was picked up by findBugs analysis > > WEB-INF/lps/server/src/org/openlaszlo/compiler/NodeModel.java: need to tell > the JDOM XML parser to preserve newlines in text. > > > test/lztest/lztest-textheight.lzx: Updated warnings to reflect current > state in different runtimes > > Tests: See LPP-8591 > > Look at text and image sizing in: > demos/lzpix/app.lzx swf,swf10,dhtml > calendar demo > amazon demo > test/lztest/lztest-textheight.lzx > > test/smoke/smokecheck > > Files: > M test/lztest/lztest-textheight.lzx > M WEB-INF/lps/lfc/kernel/swf/LzTextSprite.as > M WEB-INF/lps/lfc/kernel/dhtml/LzFontManager.js > 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/LzInputTextSprite.js > M WEB-INF/lps/lfc/kernel/swf9/LzTextSprite.as > M WEB-INF/lps/lfc/kernel/LzKernelUtils.lzs > M WEB-INF/lps/lfc/views/LzInputText.lzs > M WEB-INF/lps/lfc/views/LzText.lzs > M WEB-INF/lps/lfc/views/LaszloView.lzs > M WEB-INF/lps/server/src/org/openlaszlo/sc/CommonGenerator.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/NodeModel.java > > Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20100410-hqm-e.tar > -- Henry Minsky Software Architect [email protected]
