Bug 79914

Summary:

The initial value of text-align should be start instead of -webkit-auto

Product:

WebKit

Reporter:

Aharon (Vladimir) Lanin <aharon>

Component:

CSS

Assignee:

Ryosuke Niwa <rniwa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

atwilson, cmarcelo, darin, dglazkov, enrica, eric, gustavo, leviw, macpherson, menard, mitz, philn, playmobil, rniwa, shezbaig.wk, tkent, tony, webkit.review.bot, xan.lopez, xji

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

50910

Attachments:

Description	Flags
Patch	none
Fixed per Tony's comments	none
Archive of layout-test-results from ec2-cr-linux-02	none
Patch	tony: review+

Aharon (Vladimir) Lanin

Reported 2012-02-29 07:52:45 PST

When used under an element with an explicit text-align:start (or text-align:end), text-align:match-parent works properly. However, it does not seem to have any effect when no ancestor of the element has an explicit text-align value. Thus, data:text/html,<div dir=ltr style="text-align:start">a!<div dir=rtl style="text-align:-webkit-match-parent">b!</div></div> works properly. But data:text/html,<div dir=ltr>a!<div dir=rtl style="text-align:-webkit-match-parent">b!</div></div> does not.

Attachments
Patch (125.15 KB, patch) 2012-06-14 14:58 PDT, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Fixed per Tony's comments (128.22 KB, patch) 2012-06-14 17:04 PDT, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ec2-cr-linux-02 (817.76 KB, application/zip) 2012-06-15 02:13 PDT, WebKit Review Bot	no flags	Details
Patch (162.85 KB, patch) 2012-06-15 14:05 PDT, Ryosuke Niwa	tony: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Ryosuke Niwa

Comment 1 2012-06-13 16:13:58 PDT

I think the real bug here is the default value of text-align is "auto" instead of "start" in WebKit: http://www.w3.org/TR/css3-text/#text-align0

Ryosuke Niwa

Comment 2 2012-06-14 14:58:48 PDT

Created attachment 147658 [details] Patch

Tony Chang

Comment 3 2012-06-14 16:09:12 PDT

Comment on attachment 147658 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=147658&action=review r- because GTK compile failure. > Source/WebCore/css/StyleResolver.cpp:2065 > + // FIXME: We shouldn't be overriding start/-webkit-auto like this. Maybe mention editing html.css here? > Source/WebCore/editing/EditingStyle.cpp:913 > + if (textAlignResolvingStartAndEnd(styleAtPosition) == textAlignResolvingStartAndEnd(styleAtPosition)) This will always be true. > Source/WebCore/rendering/RenderRubyText.cpp:58 > + // FIXME: This check is bogus since user can set the initial value. > + if (textAlign != RenderStyle::initialTextAlign()) Should we file a bug for this? > LayoutTests/fast/css/text-align-webkit-auto.html:5 > window.layoutTestController.dumpAsText(); We might want to rename this test to text-align-initial.html or text-align-start.html.

Ryosuke Niwa

Comment 4 2012-06-14 17:04:16 PDT

Created attachment 147680 [details] Fixed per Tony's comments

WebKit Review Bot

Comment 5 2012-06-15 02:13:01 PDT

Comment on attachment 147680 [details] Fixed per Tony's comments Attachment 147680 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12952851 New failing tests: fast/lists/001-vertical.html editing/pasteboard/data-transfer-items.html fast/lists/001.html fast/css/list-item-text-align.html editing/pasteboard/onpaste-text-html.html

WebKit Review Bot

Comment 6 2012-06-15 02:13:07 PDT

Created attachment 147774 [details] Archive of layout-test-results from ec2-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-02 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

Tony Chang

Comment 7 2012-06-15 11:38:00 PDT

Comment on attachment 147680 [details] Fixed per Tony's comments View in context: https://bugs.webkit.org/attachment.cgi?id=147680&action=review The test failures look related. > Source/WebCore/editing/EditingStyle.cpp:377 > +template<typename T> > +static int textAlignResolvingStartAndEnd(T* style) Using a template is cute, but wouldn't it be less generated code to just pass in the results of getIdentifierValue(style, CSSPropertyDirection) and getIdentifierValue(style, CSSPropertyTextAlign) as parameters?

Ryosuke Niwa

Comment 8 2012-06-15 12:02:26 PDT

(In reply to comment #7) > (From update of attachment 147680 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=147680&action=review > > The test failures look related. Yeah, we just need to rebaseline them except list-item-text-align.html. My change to that file had gotten lost somehow :( > > Source/WebCore/editing/EditingStyle.cpp:377 > > +template<typename T> > > +static int textAlignResolvingStartAndEnd(T* style) > > Using a template is cute, but wouldn't it be less generated code to just pass in the results of getIdentifierValue(style, CSSPropertyDirection) and getIdentifierValue(style, CSSPropertyTextAlign) as parameters? Will do.

Ryosuke Niwa

Comment 9 2012-06-15 14:05:57 PDT

Created attachment 147895 [details] Patch

Ryosuke Niwa

Comment 10 2012-06-15 14:54:53 PDT

Committed r120495: <http://trac.webkit.org/changeset/120495>

Andrew Wilson

Comment 11 2012-06-15 18:07:46 PDT

It looks like this may have broken fast/lists/001-vertical.html and fast/lists/001.html: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&tests=fast%2Flists%2F001-vertical.html%2Cfast%2Flists%2F001.html

Andrew Wilson

Comment 12 2012-06-15 18:10:15 PDT

My error - only rebaselining is required.

Aharon (Vladimir) Lanin

Comment 13 2012-06-17 00:18:13 PDT

Is this bug supposed to be fixed in Chrome 21.0.1177.0 canary? The original test case (data:text/html,<div dir=ltr>a!<div dir=rtl style="text-align:-webkit-match-parent">b!</div></div>) still does not work.

Ryosuke Niwa

Comment 14 2012-06-17 12:26:14 PDT

(In reply to comment #13) > Is this bug supposed to be fixed in Chrome 21.0.1177.0 canary? The original test case (data:text/html,<div dir=ltr>a!<div dir=rtl style="text-align:-webkit-match-parent">b!</div></div>) still does not work. It works on the nightly build of WebKit for me. Wait one more week & check it again or test it using nightly builds: http://nightly.webkit.org/

Glenn Adams

Comment 15 2012-10-02 01:28:27 PDT

*** Bug 48803 has been marked as a duplicate of this bug. ***

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