[webkit-reviews] review denied: [Bug 38213] Page up / page down moves edited content for the entire height : [Attachment 56847] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue May 25 17:34:08 PDT 2010


Ojan Vafai <ojan at chromium.org> has denied Tony Chang (Google)
<tony at chromium.org>'s request for review:
Bug 38213: Page up / page down moves edited content for the entire height
https://bugs.webkit.org/show_bug.cgi?id=38213

Attachment 56847: Patch
https://bugs.webkit.org/attachment.cgi?id=56847&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
I find the horizontal centering to be a bit weird, but TextEdit does it and
it's better than the old behavior of aligning to the left. 

I would r+ except for platform/mac/editing/selection/25228.html. I feel like we
should either fix it so it's actually testing what it claims to or delete the
test. The only thing I can think of for testing this would be to do the
following:
1. Use the "ahem" font-family so that character widths are fixed. I think
they're all 15px.
2. Make the div wide (e.g. 1000px)
3. Put the cursor in the middle of the first line.
4. Move the cursor backward one character.
5. Assert that there is a specific scrollLeft value on the body.

If http://trac.webkit.org/changeset/42583 ever regressed, the scrollLeft would
change. It's harder than the old test to verify correctness visually, but at
least should keep testing that code path and we'll know if it ever changes.


More information about the webkit-reviews mailing list