Testcase and screenshot to follow. If you keep typing into the textbox until it's full and starts pushing text to the left then the top of the text gets some weird rendering glitches.

This may be just "scrolling doesn't work", which is known

More weirdness in the same testcase. The cursor is positioned at the end of the text, but it is drawn behind the last characters.

And when you try to delete the text you typed in it the removal of glyphs seems really broken. Try typing 01234567890123456 into the input and then delete the characters.

Comment on attachment 236071 [details] screenshot I've just been surprised to realize that it's the top fraction of the input that is being rendered correctly here, and the bottom majority is the part that's wrong.

Okay, here's a much better testcase with some useful observations inline.

(In reply to comment #7) > Created an attachment (id=242875) [edit] > testcase - better demonstrates the problem > > Okay, here's a much better testcase with some useful observations inline. > On the mac covering the window does not always re-render the text as described in step 1. But selecting the last letter will repaint the phrase. rest of the testcase is similar hence moving to all.

Comment on attachment 242875 [details] testcase - better demonstrates the problem In current trunk the behavior has changed a bit. The top and bottom of the text in the input only becomes disjointed on typing the first character that fills the input and causes it to scroll. Subsequent letters typed make things scroll properly. I also note that pasting text into the input will cause the disjointed top and bottom problem if the text is wider than the input. For example try pasting "123456789000000000" into the input. The top of the input will display the zeros but the bottom will display the 123...

The behavior change occurred between the 2007-01-17-04-trunk and 2007-01-19-04-trunk builds. Unfortunately the windows build from 2007-01-18-04-trunk doesn't run the testcase so I can't do better than a two day range: http://bonsai.mozilla.org/cvsquery.cgi?module=SeaMonkeyAll&date=explicit&mindate=2007-01-17&maxdate=2007-01-19+04 Quite a lot of exciting stuff going on there. Hmm, something to look more closely at later.

So the change (improvement) in the behavior of the testcase was caused by Boris'/Robert's bug/patch for bug 366001 to remove the old reflow batching API. Very curios that it didn't fix the display for the initial scroll of the input's content but fixes all subsequent scrolls. Weird.

One other series of events that causes incorrect painting is to type until the input overflows, hit ctrl-z and then type a single letter. Only the top half of the letter will paint.

Here's another testcase that shows that the invalidation area on scrolling seems to be the area that the textarea would occupy if it wasn't translated by the foreignObject's 'x' attribute. Note we create a view for the foreignObject's anonymous child, but we never position or size that view to take account of the foreignObject's x, y, width or height. I've played about with trying to set it, but either I'm doing it all wrong or that's not the problem.

We can't make view geometry account for foreignobject because the foreignobject could be transformed in ways that views can't handle. Anything that depends on views being right just won't work inside foreignobject.

So is this basically blocked on bug 337801 (Remove views) and the new mechanism views are replaced with?

I'm not sure. Plain old invalidation (not scrolling) should actually work because invalidation goes through nsIFrame::InvalidateInternal.

Wow! The second and third testcases now work! Checking the nightlies, these tests are broken in 2007-12-03-05-trunk and fixed in 2007-12-04-05-trunk. http://bonsai.mozilla.org/cvsquery.cgi?module=SeaMonkeyAll&date=explicit&mindate=2007-12-03&maxdate=2007-12-04+05%3A00

It seems the second and third testcases started working after the checkin for bug 401112 at 2007-12-03 08:57. What happened is nsSVGUtils::FindFilterInvalidation was changed to return the frames mRect if the frame is not filtered. As a result nsSVGForeignObjectFrame::FlushDirtyRegion will never invalidate anything less than the entire nsSVGForeignObjectFrame's covered area, regardless of how big or small mDirtyRegion actually is. Hence the real issue is actually just being hidden, and we're now being horribly inefficient in what we repaint. :-(

I filed bug 418063 for this perf regression.

It seems like this bug would be fixed if the view for the foreignObject's anonymous block frame would have its mPosX and mPosY members set. For some reason these are zero.

The view doesn't get set since we set mRect directly. This patch gets the second and third testcases working, but it seems really messy to be setting some dimensions on the nsSVGForeignObjectFrame and some on its anonymous block frame child. It's not clear to me why we have to set the view on the anonymous block instead of the nsSVGForeignObjectFrame (or whether we even really need the anonymous block). Of course any descendants that have views will not play nice with rotation etc., so maybe we're just best to invalidate the entire foreignObject area for any changes to descendants until we can fix that.

Comment on attachment 303869 [details] [diff] [review] patch Actually, with this patch, textarea seems to work even with rotation and skew. Cool.

I assume there's a reason that SyncFrameViewAfterReflow or some such wasn't doing what you want it to be doing?

I guess because in nsSVGForeignObjectFrame::DoReflow we're passing zero into FinishReflowChild for aX and aY.

Should we perhaps fix that? Or is our child really at 0,0 and the issue is that our _parent_ repositions us without calling FinishReflowChild? Put another way, whoever does the SetRect/DidReflow calls is responsible for invalidation and repositioning the views.

The nsSVGForeignObjectFrame's mRect is the bounding box of the foreignobject element in pixels relative to the outer <svg> element. So what this patch is doing is positioning the child's view so its origin is at the top-left of that bounding box. Well, actually no, since <svg> elements don't have views so if the <svg> is not the root element, the parent view could be anywhere and it's actually positioning the child's view at essentially a random position in the page. Even if we get lucky there, there is certainly no guarantee that invalidating view descendants of the foreignobject is going to invalidate anything like the right area of the screen if the foreignobject is transformed at all. I think for 1.9 we should go with the current behaviour that any invalidation of the foreignobject invalidates the whole thing. Performance sucks but it works OK.

The problem with that is that for some things (e.g. scrolling an area without scrollbars) all invalidation seem to go through the view tree and bypass the frame tree entirely. For the attached scrolling textarea testcase, the only reason we get a notification through the frame tree (and therefore get to invalidate the whole foreignObject area) is because that's how the scroll thumb notifies. It would probably be best to try and fix up view positioning as best we can so there's at least a chance of these classes of invalidation working for simple translation and scale transforms. I think we can find the bounding box of the foreignObject in the outer <svg>'s coordinate system, and then add the offset of the outer <svg> from its nearest view, no? What I'm not clear on is whether there will be nasty side effects of making the view's width and height different to the frame's width and height.

To answer your question Boris, I'm not really clear on whether we can fix the FinishReflowChild call to not pass in zero for aX and aY. The original author of this code is passing in NS_FRAME_NO_MOVE_FRAME to ReflowChild and FinishReflowChild, and I'm not sure what side effects we'd see if we removed that flag to allow setting of the view.

The only thing we could handle easily is a translation. Handling scale is not feasible because the view sizes need to stay in sync with frame sizes. If you want to improve the situation here for 1.9, you could try adding a state bit to views that means "I am inside a foreignobject". Then you modify scrollframes and maybe some other places to check for that bit and invalidate the frame when we try to scroll.

Keith's work on CSS transforms will probably create the view infrastructure we need here.

The testcase seems to work even when the bug 418063 bodge is removed.

WFM based on my testing and bug 418063 comment 11.