The comment at line 184 in LzTextSprite doesn't make sense any more, now that you've moved the callback into the subroutine.
Please comment at line 227 why a timeout of 0 is all that is necessary (instead of "a little more time", you are really just doing the equivalent of a "call on idle"). I don't understand what's happening at line 254: Why do you have to set scrolling again here, it should have already been set correctly, shouldn't it? Please comment at line 285 why you are always sending an update for scrollHeight. Perhaps also file a bug. I suspect that all you are doing is causing an event that causes the LZX app to recalculate. Most likely there is some other event that is missing, because clearly you should not have to send an event for an attribute that has not changed. In LzText, you changed updateLineAttribute to use ceiling and _one_ of its callers to not clamp inputs to >0. Seems to me you should change all the callers? With those fixes, approved. On 2009-11-05, at 21:59, Henry Minsky wrote: > Change 20091105-hqm-b by [email protected] on 2009-11-05 21:45:29 > EST > in /Users/hqm/openlaszlo/trunk-clean > for http://svn.openlaszlo.org/openlaszlo/trunk > > Summary: fix (mostly Firefox) DHTML text scrolling bugs > > New Features: > > Bugs Fixed: LPP-8583, LPP-8584, LPP-8585 > > Technical Reviewer: max > QA Reviewer: ptw > Doc Reviewer: (pending) > > Documentation: > > Release Notes: > > Overview: > > > Details: > > + To get properly updated size info back to the LFC, in Firefox, we > need to schedule a callback > via setTimer, to allow browser to re-layout the div. The time > doesn't seem to matter, just that the > routine is called from the timer queue. > > + LzText: use 'ceiling' when rounding to compute a line number from a > pixel size. > > + LzTextSprite: In __updatefieldprop() ( for some reason I cannot > figure out ), the value of scrollHeight *always* needs to be sent back > to the LFC, regardless of whether the kernel thinks it has changed or > not. Otherwise maxscroll never gets updated properly in LzText. I must > be > missing something subtle about how setHeight is working. > > > + misc unrelated fix for makeTextLink > > > Tests: > > + see new testcase maxscroll.lzx, the bug was that you were unable to > scroll to the end of the text view using setAttribute('scroll', > this.maxscroll). > > Test procedure > > [1] click red square to scroll text view to end it's maxscroll > value > > [2] click "set field height to 100" button to resize the height, and > then click red button, and make sure last line of text, "32 > THIS IS > THE LAST LINE!", is still displayed at bottom of text view > > [3] use slider to change height of text view, make sure last line is > always visible displayed at bottom of view. > > > Files: > A test/lfc/maxscroll.lzx > M WEB-INF/lps/lfc/kernel/dhtml/LzTextSprite.js > M WEB-INF/lps/lfc/views/LzText.lzs > > Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20091105-hqm-b.tar > _______________________________________________ > Laszlo-reviews mailing list > [email protected] > http://www.openlaszlo.org/mailman/listinfo/laszlo-reviews _______________________________________________ Laszlo-reviews mailing list [email protected] http://www.openlaszlo.org/mailman/listinfo/laszlo-reviews
