Bug 138250

Summary: Get rid of ruby base and text overlapping when selected and make gaps above ruby text fill in
Product: WebKit Reporter: Dave Hyatt <hyatt>
Component: Layout and RenderingAssignee: Dave Hyatt <hyatt>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, esprehn+autocc, glenn, kondapallykalyan, mmaxfield, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
bfulgham: review+
Patch
none
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion
none
Patch
dino: review+, buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2
none
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion none

Description Dave Hyatt 2014-10-31 08:50:58 PDT
Get rid of ruby base and text overlapping when selected and make gaps above ruby text fill in.
Comment 1 Dave Hyatt 2014-10-31 08:53:22 PDT
Created attachment 240735 [details]
Patch
Comment 2 Dave Hyatt 2014-10-31 08:57:35 PDT
Created attachment 240736 [details]
Patch
Comment 3 Brent Fulgham 2014-10-31 09:09:32 PDT
Comment on attachment 240735 [details]
Patch

r=me, assuming tests pass. EWS hasn't quite finished chewing on this yet.
Comment 4 Build Bot 2014-10-31 10:46:23 PDT
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
Comment 5 Build Bot 2014-10-31 11:12:12 PDT
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
Comment 6 Myles C. Maxfield 2014-11-03 09:59:17 PST
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
Comment 7 Dave Hyatt 2014-11-17 14:21:42 PST
Created attachment 241742 [details]
Patch
Comment 8 Dean Jackson 2014-11-17 14:42:46 PST
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.
Comment 9 Build Bot 2014-11-17 15:40:29 PST
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
Comment 10 Build Bot 2014-11-17 15:40:34 PST
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
Comment 11 Build Bot 2014-11-17 16:03:55 PST
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
Comment 12 Build Bot 2014-11-17 16:04:05 PST
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
Comment 13 Dave Hyatt 2014-11-18 09:59:28 PST
Fixed.