[webkit-reviews] review denied: [Bug 3749] XHTML 1.1 Ruby Annotation : [Attachment 32823] patch: HTML5 ruby functionality only - requires patch in bug #32822 for build option

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Roland Steiner
<rolandsteiner at google.com>'s request for review:
Bug 3749: XHTML 1.1 Ruby  Annotation
https://bugs.webkit.org/show_bug.cgi?id=3749

Attachment 32823: patch: HTML5 ruby functionality only - requires patch in bug
#32822 for build option
https://bugs.webkit.org/attachment.cgi?id=32823&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
Why an extra ruby stylesheet?  I think the rules can just go in html.css. 
There are only 4!

ruby { display: inline } is unnecessary.  The default display type is inline,
so you don't need that rule.

Can rp just always be display:none?

When does rt occur outside a ruby?

There is funny indentation in InlineFlowBox.h... it looks like you 3-space
indented instead of 4.

I do not like the addition of an extra virtual function call in
InlineFlowBox.cpp's placeBoxesVertically method.  Possible to avoid it?

In RenderBlock.cpp I worked hard to devirtualize the container walk, so having
it be re-virtualized is bothersome.  I'm curious why that is needed.  It looks
like you're using some CSS style constant to scramble order.  Is that actually
part of HTML5?