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: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | All | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 50910 | ||||||||||||
Attachments: |
|
Description
Aharon (Vladimir) Lanin
2012-02-29 07:52:45 PST
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 Created attachment 147658 [details]
Patch
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. Created attachment 147680 [details]
Fixed per Tony's comments
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 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
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? (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. Created attachment 147895 [details]
Patch
Committed r120495: <http://trac.webkit.org/changeset/120495> 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 My error - only rebaselining is required. 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. (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/ *** Bug 48803 has been marked as a duplicate of this bug. *** |