[Webkit-unassigned] [Bug 96592] Ruby text is incorrectly positioned when its writing-mode is changed to vertical after layout is done

Wed Oct 31 23:01:02 PDT 2012

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

--- Comment #6 from Yuki Sekiguchi <yuki.sekiguchi at access-company.com>  2012-10-31 23:02:23 PST ---
Thank you for reviewing, Morrita-san!

(In reply to comment #4)
> > Source/WebCore/rendering/RenderRubyRun.cpp:238
> > +    // Logical left of RenderRubyText should always be 0.
> 
> We don't need this line. The comment says something obvious from the code.

Removed.

> > LayoutTests/ChangeLog:8
> > +        Update LayoutTest to use testRunner not layoutTestController
> 
> This explanation doesn't looks making sense. I'd rather drop this.

Removed and add description of test case.

> > LayoutTests/ChangeLog:15
> > +        RenderRubyText::y remain old one.
> 
> We don't need to duplicate this. It's sufficient to have the explanation in  WebCore/ChangeLog .

Removed.

> > LayoutTests/fast/writing-mode/ruby-text-logical-left-expected.html:3
> > +<style>
> 
> Please put <sytle> in <head> unless there is reason to do it.
> Also, please make CSS syntax valid by adding ";"

Fixed.

> > LayoutTests/fast/writing-mode/ruby-text-logical-left-expected.html:10
> > +</body></html>
> 
> It would be great if you have more coverage.
> - What happens if the style is removed?
> - What if ruby has wider width than decorated text?

Add test cases.

> > LayoutTests/fast/writing-mode/ruby-text-logical-left.html:11
> > +  setTimeout(function () {
> 
> Please indent the code appropriately.

Fixed.

> > LayoutTests/fast/writing-mode/ruby-text-logical-left.html:14
> > +  if (testRunner) {
> 
> Do check window.testRunner instead of just referring testRunner.
> In that way we can run the test not only on DRT but also in a plain browser.

I understand it and fix it.

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