Bug 138250 - Get rid of ruby base and text overlapping when selected and make gaps above ruby text fill in
Summary: Get rid of ruby base and text overlapping when selected and make gaps above r...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-31 08:50 PDT by Dave Hyatt
Modified: 2014-11-18 09:59 PST (History)
7 users (show)

See Also:


Attachments
Patch (10.47 KB, patch)
2014-10-31 08:53 PDT, Dave Hyatt
bfulgham: review+
Details | Formatted Diff | Diff
Patch (11.44 KB, patch)
2014-10-31 08:57 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (743.26 KB, application/zip)
2014-10-31 10:46 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-03 for mac-mountainlion (716.95 KB, application/zip)
2014-10-31 11:12 PDT, Build Bot
no flags Details
Patch (14.31 KB, patch)
2014-11-17 14:21 PST, Dave Hyatt
dino: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (586.40 KB, application/zip)
2014-11-17 15:40 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (557.42 KB, application/zip)
2014-11-17 16:04 PST, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
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.