[webkit-reviews] review requested: [Bug 23556] Right-to-left pages should be scrollable to reveal left overflow : [Attachment 74233] patch

[bugzilla-daemon at webkit.org]

Jeremy Moskovich <playmobil at google.com> has asked  for review:
Bug 23556: Right-to-left pages should be scrollable to reveal left overflow
https://bugs.webkit.org/show_bug.cgi?id=23556

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

------- Additional Comments from Jeremy Moskovich <playmobil at google.com>
dhyatt,mitz: Could you take a look please?

Updated patch with following changes:
1)History restore now works correctly, see note below.
2) Minor cleanup of horizontal-scrollbar-in-rtl.html.
3) Added layout test to make sure no scroll events are fired on initial
display.

I chatted with dhyatt a bit on irc on how the platformWidget code should update
the initial scrollbar position.  The logical option would be to call
FrameView:: setScrollPosition(), however the only place where we know that a
horizontal scrollbar has been created is in [WebDynamicScrollView
updateScrollers] and calling out to FrameView from there would be a layering
violation.  The other problem is that FrameView:: setScrollPosition() triggers
a JS scroll event, which is not something we want to do when setting the
initial scrollbar position.

The patch moves the scrollers by calling [documentView scrollPoint:] and works
around the problem in comment #57 by short-circuiting the notification callback
if it detects that the scroller is having it's initial position set
programmatically.

If you think there's a better way of doing this, I'd appreciate a suggestion
for an alternate approach.

Known issues with this patch:
1) scrollbar location not updated when programmatically setting direction for
platformWidget code.
2) resize test in horizontal-scrollbar-in-rtl.html  fails on mac platform.

I think that neither of these are regressions from the current behavior, and if
it's OK I'd like to address them in a followup patch.