Bug 31247

Summary:

text-indent is unexpectedly applied to <ruby>

Product:

WebKit

Reporter:

Kent Tamura <tkent>

Component:

Layout and Rendering

Assignee:

Roland Steiner <rolandsteiner>

Status:

RESOLVED FIXED

Severity:

Normal

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
Example HTML	none
Rendering result with the latest WebKit + Safari	none
patch - reset 'text-indent' on <ruby> and <rt> elements	none
patch - reset 'text-indent' on <ruby> and <rt> elements, change layout test to use dumpAsText()	darin: review+, commit-queue: commit-queue-

Kent Tamura

Reported 2009-11-08 19:11:32 PST

Created attachment 42726 [details] Example HTML Please see the attachments. There should be no space at the left of the <ruby> elements.

Attachments
Example HTML (257 bytes, text/html) 2009-11-08 19:11 PST, Kent Tamura	no flags	Details
Rendering result with the latest WebKit + Safari (10.63 KB, image/png) 2009-11-08 19:12 PST, Kent Tamura	no flags	Details
patch - reset 'text-indent' on <ruby> and <rt> elements (38.36 KB, patch) 2009-11-18 01:20 PST, Roland Steiner	no flags	Details Formatted Diff Diff
patch - reset 'text-indent' on <ruby> and <rt> elements, change layout test to use dumpAsText() (3.87 KB, patch) 2009-11-18 20:47 PST, Roland Steiner	darin: review+ commit-queue: commit-queue-	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Kent Tamura

Comment 1 2009-11-08 19:12:19 PST

Created attachment 42727 [details] Rendering result with the latest WebKit + Safari

Roland Steiner

Comment 2 2009-11-18 01:20:01 PST

Created attachment 43412 [details] patch - reset 'text-indent' on <ruby> and <rt> elements The ruby uses block objects to align the ruby text with its base: RenderRubyRun (anonymous, wrapping a base and a text), RenderRubyBase (anonymous, containing the base text) and RenderRubyText (<rt>, containing the annotation text). RenderRuby, corresponding to the <ruby> element is an inline renderer. However, <ruby> is just a means to add annotations to a base text and thus not seen to open a block. Therefore, text-indent should not apply. Thus, the patch changes the default UA style-sheet to reset 'text-indent' to 0. I was considering whether a hard-coded approach to take out the "block-ness" wouldn't be better, but at least for now I went with the KISS principle. There is also the question whether we we need to consider other CSS constructs that may have similar effects, such as :first-line (?). However, as a pseudo-element this needs a selector subject, so AFAICT you'd need some contrived CSS though for those to effect ruby in an non-intended way. Therefore I left the patch with only addressing text-indent for the time being.

Eric Seidel (no email)

Comment 3 2009-11-18 15:51:34 PST

Comment on attachment 43412 [details] patch - reset 'text-indent' on <ruby> and <rt> elements Can't this be a dumpAsText test? Since all we need to do is check the getComputedStyle(element).textIndent value? Also, we don't need this extra stuff at the top of the html file: +<head> +<style> +</style> +</head> +<body> This will cause non-mac platforms to fail since it only has mac results. That's OK, as we can update them after commit, but it would be better if we could just make this dumpAsText and avoid the trouble all together.

Roland Steiner

Comment 4 2009-11-18 20:47:23 PST

Created attachment 43480 [details] patch - reset 'text-indent' on <ruby> and <rt> elements, change layout test to use dumpAsText() I guess I'm just more of a visual person... But you're right of course that a full pixel test isn't really necessary here - changed the layout test to use dumpAsText().

WebKit Commit Bot

Comment 5 2009-11-25 00:08:52 PST

Comment on attachment 43480 [details] patch - reset 'text-indent' on <ruby> and <rt> elements, change layout test to use dumpAsText() Rejecting patch 43480 from commit-queue. Failed to run "['/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', '--reviewer', 'Darin Adler', '--force']" exit_code: 1 patching file WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file WebCore/css/html.css Hunk #1 FAILED at 609. 1 out of 1 hunk FAILED -- saving rejects to file WebCore/css/html.css.rej patching file LayoutTests/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file LayoutTests/fast/ruby/ruby-text-indent-expected.txt patching file LayoutTests/fast/ruby/ruby-text-indent.html

Roland Steiner

Comment 6 2009-11-25 01:21:10 PST

(In reply to comment #5) Oops, sorry - I had already committed the patch.

Kent Tamura

Comment 7 2009-11-25 01:40:05 PST

Oh, I see. It's r51227.

Note You need to log in before you can comment on or make changes to this bug.