Bug 138250 – Get rid of ruby base and text overlapping when selected and make gaps above ruby text fill in

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Dave Hyatt]

Get rid of ruby base and text overlapping when selected and make gaps above ruby text fill in.

[Dave Hyatt]

Created attachment 240735 [details]
Patch

[Dave Hyatt]

Created attachment 240736 [details]
Patch

[Brent Fulgham]

Comment on attachment 240735 [details]
Patch

r=me, assuming tests pass. EWS hasn't quite finished chewing on this yet.

[Build Bot]

Created attachment 240743 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Created attachment 240744 [details]
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-03  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Myles C. Maxfield]

Comment on attachment 240736 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=240736&action=review

Unofficial r=me

> Source/WebCore/rendering/RootInlineBox.cpp:600
> +    

:(

> Source/WebCore/rendering/RootInlineBox.cpp:612
> +            RenderRubyBase* base = run->rubyBase();

You've already got the ruby base, no need to ask the run for it

> Source/WebCore/rendering/RootInlineBox.cpp:622
> +            RenderRubyText* text = run->rubyText();

Ditto

> Source/WebCore/rendering/RootInlineBox.cpp:687
> +    if (renderer().isRubyBase()) {

Too bad we can't have code sharing between these two sections

[Dave Hyatt]

Created attachment 241742 [details]
Patch

[Dean Jackson]

Comment on attachment 241742 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=241742&action=review

> Source/WebCore/rendering/RootInlineBox.cpp:600
> -
> +    

Oops.

> Source/WebCore/rendering/RootInlineBox.cpp:612
> +            RenderRubyBase* base = run->rubyBase();

From Myles: you already have the ruby base.

> Source/WebCore/rendering/RootInlineBox.cpp:683
> -
> +    

Oops.

[Build Bot]

Comment on attachment 241742 [details]
Patch

Attachment 241742 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5344205148782592

New failing tests:
fast/ruby/modify-positioned-ruby-text-crash.html
fast/ruby/rubyDOM-remove-text2.html
fast/ruby/ruby-empty-rt.html
fast/css/bidi-override-in-anonymous-block.html

[Build Bot]

Created attachment 241744 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 241742 [details]
Patch

Attachment 241742 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6624714214604800

New failing tests:
fast/ruby/modify-positioned-ruby-text-crash.html
fast/ruby/rubyDOM-remove-text2.html
fast/ruby/ruby-empty-rt.html

[Build Bot]

Created attachment 241745 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Dave Hyatt]

Fixed.