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

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Aharon (Vladimir) Lanin]

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.

[Ryosuke Niwa]

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]

Created attachment 147658 [details]
Patch

[Tony Chang]

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]

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

[WebKit Review Bot]

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]

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 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]

(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]

Created attachment 147895 [details]
Patch

[Ryosuke Niwa]

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

[Andrew Wilson]

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]

My error - only rebaselining is required.

[Aharon (Vladimir) Lanin]

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]

(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]

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