User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b3) Gecko/20050712 Firefox/1.0+ In Deerpark alpha 2, a overflowing multiline textbox doesn't seem to trigger the onoverflow event handler. I'll attach a simple XUL file that I used to test this. Reproducible: Always Steps to Reproduce: 1. Open the attached XUL page 2. Hit enter until in textbox until scrollbar appears 3. Note difference in behavior in Firefox 1.0 and Deerpark alpha 2 Actual Results: In deerpark alpha 2 the label below the textbox doesn't change from "No overflow yet". Expected Results: In Firefox 1.0 the label below the textbox changes to "Overflow detected" when the textbox's contents are too tall for the box and to "Underflow detected" when the textbox's contents fit in the textbox. This seems like an issue that would have been discovered already, but I couldn't find anything in bugzilla. However this is my first ticket filed, so I might not be searching correctly. My apologies in advance if this is a dupe.

Bug seen in: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20050813 Firefox/1.0+ Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20050815 SeaMonkey/1.0a working: Mozilla/5.0 (Windows; U; Win98; de-DE; rv:1.7.10) Gecko/20050715 Firefox/1.0.6 Working from Seamonkey, I first tested Firefox 1.0.6, then Seamonkey, and Seamonkey crashed after I closed Firwefox, started Deerpark and wanted to copy&paste the testcase URL from Seamonkey to Deerpark. The bug is about a textbox, and I copied the UA of the browser tested to this Additional Comments textbox in the bug I was working on in a tab in Seamonkey. Talkback TB8496600G, DocWatson tells it crashed in GKWIDGET.DLL, may be related or not. Excerpt from testcase: <textbox id="overflow-textbox" multiline="true" rows="2" onoverflow="document.getElementById('overflow-results').value = 'Overflow dected';" onunderflow="document.getElementById('overflow-results').value = 'Underflow dected';" /> <label id="overflow-results" value="No overflow yet" /> I can write three lines of text, first two including return, third without. When I hit the return key after third line is written, the textbox scrolls so only a few pixels of first line are seen, the 2nd and 3rd line are seen perfectly, cursor is on an empty 4th line, and a vertical scrollbar has appeared to the right. Firefox 1..0.6 shows same behaviour, but hitting the 3rd return changes text from 'no overflow yet' to 'overflow detected'

This worked in 2005-04-28 build, but fails in 2005-04-29 build, I think a regression from fixing bug 289940.

Regression in 1.0.5+ and Deer Park, nominating. /be

I suspect is actually fallout from bug 240276. XUL scrollframes still post the overflow events, but <textbox> uses an <html:textarea> as the editing area, and I'm pretty sure that uses an HTML scrollframe (for its anonymous div) as the scrollframe.

Gah. I guess I'll have to put that back.

Untested! I'm building but I just want to post this before I leave.

Comment on attachment 193412 [details] [diff] [review] fix OK, I tested this and it works. I've changed the behaviour of the overflow event so that it only fires when the state changes from "not overflowing" to "overflowing". This is more efficient, consistent with the handling of "underflow" and shouldn't cause any problems ... the only user of this event in our tree that I know of (menu scrolling) works fine with that, and the only other users are presumably a few XUL authors.

Comment on attachment 193412 [details] [diff] [review] fix This chunk (with the "-" lines omitted): > // first see what changed > PRBool vertChanged = PR_FALSE; > PRBool horizChanged = PR_FALSE; > >+ if (mVerticalOverflow && childSize.height <= scrollportSize.height) { > mVerticalOverflow = PR_FALSE; > vertChanged = PR_TRUE; >+ } else if (!mVerticalOverflow && childSize.height > scrollportSize.height) { >+ mVerticalOverflow = PR_TRUE; > vertChanged = PR_TRUE; > } > >+ if (mHorizontalOverflow && childSize.width <= scrollportSize.width) { > mHorizontalOverflow = PR_FALSE; > horizChanged = PR_TRUE; >+ } else if (!mHorizontalOverflow && childSize.width > scrollportSize.width) { >+ mHorizontalOverflow = PR_TRUE; > horizChanged = PR_TRUE; > } can now (thanks to your changes) be simplified to: PRBool newVerticalOverflow = childSize.height > scrollportSize.height; PRBool vertChanged = mVerticalOverflow != newVerticalOverflow; mVerticalOverflow = newVerticalOverflow; PRBool newHorizontalOverflow = childSize.width > scrollportSize.width; PRBool horizChanged = mHorizontalOverflow != newHorizontalOverflow; mHorizontalOverflow = newHorizontalOverflow;

Comment on attachment 193412 [details] [diff] [review] fix >+ if (vertChanged && horizChanged) { >+ if (mVerticalOverflow == mHorizontalOverflow) >+ { >+ // both either overflowed or underflowed. 1 event >+ PostScrollPortEvent(mVerticalOverflow, nsScrollPortEvent::both); >+ } else { >+ // one overflowed and one underflowed >+ PostScrollPortEvent(mVerticalOverflow, nsScrollPortEvent::vertical); >+ PostScrollPortEvent(mHorizontalOverflow, nsScrollPortEvent::horizontal); >+ } >+ } else if (vertChanged) { // only one changed either vert or horiz >+ PostScrollPortEvent(mVerticalOverflow, nsScrollPortEvent::vertical); >+ } else if (horizChanged) { >+ PostScrollPortEvent(mHorizontalOverflow, nsScrollPortEvent::horizontal); > } I tend to prefer if (A) { if (B) { // 1 } else { // 2 } } else { if (B) { // 3 } } over if (A && B) { // 1 } else if (A) { // 2 } else if (B) { // 3 } but that's probably over-optimizing, so it's mainly which style you prefer. Either's fine with me. (But I do think the simplification in my previous comment makes things quite a bit clearer.) Please comment that these are state used only by PostScrollEvents so it knows whether the overflow state changes. >+ PRPackedBool mHorizontalOverflow:1; >+ PRPackedBool mVerticalOverflow:1; r+sr=dbaron

checked in with David's comments.

Comment on attachment 193412 [details] [diff] [review] fix Fairly safe fix to make the branch compatible with 1.7 and trunk

checked into branch

Is this relevant to 1.0.x branch?

> Is this relevant to 1.0.x branch? No.