Bug 13375
| Summary: | REGRESSION (r20901): failing fast/overflow/scrollRevealButton.html | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | mitz |
| Component: | WebCore Misc. | Assignee: | Darin Adler <darin> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | darin |
| Priority: | P1 | Keywords: | LayoutTestFailure, Regression |
| Version: | 523.x (Safari 3) | ||
| Hardware: | Mac | ||
| OS: | OS X 10.4 | ||
mitz
fast/overflow/scrollRevealButton.html fails to scroll the button into view when it's focused.
Regressed in <http://trac.webkit.org/projects/webkit/changeset/20901>.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Darin Adler
Wow, did this change really cause the regression? I have no idea how!
mitz
(In reply to comment #1)
> Wow, did this change really cause the regression?
Yes.
> I have no idea how!
I suspect it's the removal of the updateLayout() call from setIsActive(), that just happened to be at right place at the right time to ensure that everything is set up for scroll-to-reveal to do its thing. Without it, some of the dimensions are (0, 0) which messes up the scrolling. Even prior to r20901, opening the test in a background tab, for example, would make it fail.
I've readded an updateLayout() back in order to test my theory. If it's right, then that call should probably be added in a more appropriate place to ensure that scroll-to-reveal has all the information it needs.
mitz
(In reply to comment #2)
> I suspect it's the removal of the updateLayout() call from setIsActive().
Confirmed.
Darin Adler
(In reply to comment #3)
> Confirmed.
To me it seems fine to just add an updateLayout to setIsActive, check that in, and then later try to come up with a better way to fix this problem.
mitz
Fixed by Darin in r20936.