[Webkit-unassigned] [Bug 38213] Page up / page down moves edited content for the entire height

[bugzilla-daemon at webkit.org]

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


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #54491|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #10 from David Levin <levin at chromium.org>  2010-04-29 18:37:34 PST ---
(From update of attachment 54491)
This looks like it is heading on a good path. I have some general comments and
when they are done either I or someone else should verify the patch in greater
detail.

Ideally, the ChangeLog should have per function comments to explain why the
change was done in that function.

For example, I don't know why the code in verticalScrollDistance changed and
the ChangeLog should explain this.

When you find yourself adding a bool and then passing in true/false for it, it
is a bad sign. It is especially bad if it is with a bunch of other bools.
Please change the bool to an enum that describes the desired behavior
(something like alignToTop or alignToEdgeIfNeeded?).

Also, it looks like Tony had a bunch of good comments which should be
addressed.

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