[webkit-reviews] review denied: [Bug 23556] Right-to-left pages should be scrollable to reveal left overflow : [Attachment 66128] patch w/ layout test

Fri Oct 22 15:29:47 PDT 2010

Dave Hyatt <hyatt at apple.com> has denied Xiaomei Ji <xji at chromium.org>'s request
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 66128: patch w/ layout test
https://bugs.webkit.org/attachment.cgi?id=66128&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
The scroll stuff looks fine.  I don't like the m_bodyIsLTR variable though.  If
a different direction is explicitly specified on the root element, this should
supercede the body's direction.  In addition, XML documents will typically set
the direction on the root.

Since the time this patch has been worked on, I committed a fix that propagates
both direction and writing-mode on the root element (<html>) up to the
RenderView's style.  This means the RenderView itself now matches the direction
and writing-mode of the <html>.  Rather than doing this m_bodyIsLTR hack, I
think we should probably expand the propagation code I added to also propagate
from the body.	It should only do this though if the <html> element didn't
explicitly have its direction set.

You can see this code in CSSStyleSelector::styleForDocument and then it also
dynamically updates the style in styleDidChange.

You may want to consider an initial checkin that just works with the
RenderView's direction (using test cases that set RTL on <html>) and work on
the propagation code from the <body> in a followup patch.