[Webkit-unassigned] [Bug 15135] REGRESSION (r19843-r19850): Changing a flexbox's contents makes its container scroll to the top

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=15135





------- Comment #12 from hamaji at chromium.org  2009-06-19 00:55 PDT -------
(In reply to comment #10)
> (From update of attachment 31284 [review])
> You've picked a very challenging bug to work on here.

Yeah, it was more difficult than I expected :( Sorry and thanks for your time
to check my wrong patches.

> I'm not sure I'd worry so much about repaint being wrong.  If the flexing block
> has kids that move during the first layout, your patch isn't going to help that
> case.  They are still going to repaint too much.  I think you should just
> concentrate on the scrolling issue and ignore repaint for now.
> 
> As for scrolling, trying to stop just the immediate child block from updating
> its scroll info during the first layout doesn't really generally solve the
> problem.  What about a scrollable object inside a flexing block that
> incorrectly adjusts its scroll position in response to the first layout?

Ah yes, I didn't care about grandchild nodes at all.

> I think a simpler fix for this bug is to just make the scroll position updating
> code immune to flex box layout thrashing.

I'm not sure if I understand what you are suggesting here correctly. I tried
two ways to fix only scrollbar, but both attempts failed so far :( I tried

- Just disable RenderLayer::updateScrollInfoAfterLayout() while layouting
descendants of flexible box: Scrollbars are never painted. Maybe
updateScrollInfoAfterLayout() is painting as well as updating?
- Disable updateScrollInfoAfterLayout(), and after layout descendants of the
flexible box, call updateScrollInfoAfterLayout() for delayed objects: This
means that we update scrollbar after repainting. This change seems to break
some existing layout tests.

I also tried to create a patch which fixes both update of scrollbar and
repainting (the uploaded patch), and it seems to be working. I also added
layout tests for nested flexible boxes, which my previous patch didn't handle
properly.

In this change, we skip all update of scrollbar and repaint during layout of
flexible boxes, and keep the skipped objects and contexts into global-static
hashmap (so that we don't increase the size of RenderBlock). Then, after the
layout of descendants of a flexible box finishes, we process all delayed
repainting. When we are layouting nested flexible boxes, we don't process the
delayed repainting until the first flexible finishes to layout its children.

If there are something I'm still missing, and/or you still think I should
concentrate on the scrolling, I'm happy to do so, of course. Please let me know
in this case.

Thanks!


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.