Bug 76173

Please see me, I am a CSS3 feature :D

I will look at this issue. ;-)

Created attachment 167707 [details] WIP patch This is the WIP patch for the text-align-last property. I'm trying to prepare some valuable test cases for this property now. If anyone available, please review this patch and give some comments to me. It will be very helpful to me. Thanks.

Attachment 167707 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/css/CSSParser.cpp:1767: Tab found; better to use spaces [whitespace/tab] [1] Source/WebCore/rendering/style/RenderStyle.h:260: _text_align_last is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 2 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 167707 [details] WIP patch Attachment 167707 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/14214647

Comment on attachment 167707 [details] WIP patch Attachment 167707 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/14215670

Created attachment 167711 [details] WIP patch

(In reply to comment #4) > Source/WebCore/rendering/style/RenderStyle.h:260: _text_align_last is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] There are many identifiers which use underscore in the RenderStyle.h file. Is it ok to file a new bug to fix those by myself?

Comment on attachment 167711 [details] WIP patch Attachment 167711 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14218621

Comment on attachment 167711 [details] WIP patch Attachment 167711 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14238211

Comment on attachment 167711 [details] WIP patch Attachment 167711 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/14215676

Comment on attachment 167711 [details] WIP patch Attachment 167711 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14223605

Comment on attachment 167711 [details] WIP patch Attachment 167711 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/14223608

Created attachment 167738 [details] WIP patch WIP patch This is the WIP patch for the text-align-last property. I'm trying to prepare some valuable test cases for this property now. If anyone available, please review this patch and give some comments to me. It will be very helpful to me. Thanks.

Comment on attachment 167738 [details] WIP patch Attachment 167738 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14209872

Comment on attachment 167738 [details] WIP patch Attachment 167738 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14217719

Comment on attachment 167738 [details] WIP patch Attachment 167738 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14207889

Comment on attachment 167738 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=167738&action=review > Source/WebCore/css/CSSParser.cpp:1771 > + break; You want to put this case in isValidKeywordPropertyAndValue as it will enjoy fast path from JS in some cases.

Created attachment 168175 [details] WIP patch

Comment on attachment 168175 [details] WIP patch Attachment 168175 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14245861

Comment on attachment 168175 [details] WIP patch Attachment 168175 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14257460

Comment on attachment 168175 [details] WIP patch Attachment 168175 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14255512

Created attachment 168363 [details] patch without test case I've tried to build this on mac, it have been succeeded. I believe this patch will pass the ews this time.

Created attachment 168364 [details] test case I made a test case for this property. Please see this is enough.

Do I need to make the expected results for all ports? Or, can I add this test case as a skipped case for all port except EFL port which is the major port for me?

These are my proposes! :) Could you borrow some tests from the Mozilla bug (https://bugzilla.mozilla.org/show_bug.cgi?id=536557 https://bug536557.bugzilla.mozilla.org/attachment.cgi?id=533614), Mozilla tests are under Public Domain (http://wiki.whatwg.org/wiki/Testsuite) so it would not be a problem if you want completely copy them! Their tests contain RTL text (hebrew, arabic, persian, ...) testing that is really important. Also please check your patch with http://ie.microsoft.com/testdrive/Performance/TextJustificationAnimated/Default.html the first one is related to this bug and second one to https://bugs.webkit.org/show_bug.cgi?id=20606 that is of-course a separate bug :)

(In reply to comment #26) > These are my proposes! :) > Could you borrow some tests from the Mozilla bug (https://bugzilla.mozilla.org/show_bug.cgi?id=536557 https://bug536557.bugzilla.mozilla.org/attachment.cgi?id=533614), Mozilla tests are under Public Domain (http://wiki.whatwg.org/wiki/Testsuite) so it would not be a problem if you want completely copy them! Their tests contain RTL text (hebrew, arabic, persian, ...) testing that is really important. > Also please check your patch with http://ie.microsoft.com/testdrive/Performance/TextJustificationAnimated/Default.html the first one is related to this bug and second one to https://bugs.webkit.org/show_bug.cgi?id=20606 that is of-course a separate bug :) Oh. You've worked for this property on Mozilla! That's nice! :) 1. http://ie.microsoft.com/testdrive/Performance/TextJustificationAnimated/Default.html is working fine. 2. Test cases in https://bug536557.bugzilla.mozilla.org/attachment.cgi?id=533614 are quite a lot. Do you really think we need to applying all of those in this patch?

(In reply to comment #27) > (In reply to comment #26) > > These are my proposes! :) > > Could you borrow some tests from the Mozilla bug (https://bugzilla.mozilla.org/show_bug.cgi?id=536557 https://bug536557.bugzilla.mozilla.org/attachment.cgi?id=533614), Mozilla tests are under Public Domain (http://wiki.whatwg.org/wiki/Testsuite) so it would not be a problem if you want completely copy them! Their tests contain RTL text (hebrew, arabic, persian, ...) testing that is really important. > > Also please check your patch with http://ie.microsoft.com/testdrive/Performance/TextJustificationAnimated/Default.html the first one is related to this bug and second one to https://bugs.webkit.org/show_bug.cgi?id=20606 that is of-course a separate bug :) > > Oh. You've worked for this property on Mozilla! That's nice! :) > > 1. http://ie.microsoft.com/testdrive/Performance/TextJustificationAnimated/Default.html is working fine. > > 2. Test cases in https://bug536557.bugzilla.mozilla.org/attachment.cgi?id=533614 are quite a lot. > Do you really think we need to applying all of those in this patch? And, RTL cases are also included in my test case. ;-)

(In reply to comment #28) > (In reply to comment #27) > > (In reply to comment #26) > > > These are my proposes! :) > > > Could you borrow some tests from the Mozilla bug (https://bugzilla.mozilla.org/show_bug.cgi?id=536557 https://bug536557.bugzilla.mozilla.org/attachment.cgi?id=533614), Mozilla tests are under Public Domain (http://wiki.whatwg.org/wiki/Testsuite) so it would not be a problem if you want completely copy them! Their tests contain RTL text (hebrew, arabic, persian, ...) testing that is really important. > > > Also please check your patch with http://ie.microsoft.com/testdrive/Performance/TextJustificationAnimated/Default.html the first one is related to this bug and second one to https://bugs.webkit.org/show_bug.cgi?id=20606 that is of-course a separate bug :) > > > > Oh. You've worked for this property on Mozilla! That's nice! :) > > > > 1. http://ie.microsoft.com/testdrive/Performance/TextJustificationAnimated/Default.html is working fine. > > > > 2. Test cases in https://bug536557.bugzilla.mozilla.org/attachment.cgi?id=533614 are quite a lot. > > Do you really think we need to applying all of those in this patch? > > And, RTL cases are also included in my test case. ;-) Thanks! :) IMHO it is enough if you tested Hebrew text tests manually and adding in tests is not very important (but would be cool)! Also another test that I proposed for Mozilla was this ( https://bugzilla.mozilla.org/show_bug.cgi?id=536557#c3 ) you can test it on this page http://fa.wikipedia.org/w/index.php?oldid=8222219#Test2 after installing this font http://www.scict.ir/portal/File/ShowFile.aspx?ID=bea5ca36-1fdf-41d4-8818-c1a4f9c71081 expected difference before/after the patch must be https://bug536557.bugzilla.mozilla.org/attachment.cgi?id=534295 but please don't stop committing your patch for this test because for example font render engine on Chrome Linux can not render Nastaliq well and this is obviously is a separate bug :)

(In reply to comment #29) > Thanks! :) IMHO it is enough if you tested Hebrew text tests manually and adding in tests is not very important (but would be cool)! Also another test that I proposed for Mozilla was this ( https://bugzilla.mozilla.org/show_bug.cgi?id=536557#c3 ) you can test it on this page http://fa.wikipedia.org/w/index.php?oldid=8222219#Test2 after installing this font http://www.scict.ir/portal/File/ShowFile.aspx?ID=bea5ca36-1fdf-41d4-8818-c1a4f9c71081 expected difference before/after the patch must be https://bug536557.bugzilla.mozilla.org/attachment.cgi?id=534295 but please don't stop committing your patch for this test because for example font render engine on Chrome Linux can not render Nastaliq well and this is obviously is a separate bug :) I've tested with the text-align-last-justify.html and text-align-last-justify-rtl.html in the Mozilla test cases, and it works properly.

Created attachment 168699 [details] patch I've added total 8 test cases. 7 test cases - each case tests one of the value of text-align-last property. one more case - text-align-last:justify for one line, and Hebrew character also.

Created attachment 168703 [details] patch I re-based the patch.

Attachment 168703 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 LayoutTests/platform/mac/fast/css/text-align-last-end-expected.png:0: Have to enable auto props in the subversion config file (/Users/eseidel/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/Users/eseidel/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5] Total errors found: 1 in 44 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 168824 [details] Patch

Attachment 168824 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 WARNING: Patch's size is 82 kbytes. Patches 20k or smaller are more likely to be reviewed. Larger patches may sit unreviewed for a long time. LayoutTests/platform/efl/fast/css/text-align-last-auto-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/efl/fast/css/text-align-last-auto-expected.png). [image/png] [5] LayoutTests/platform/mac/fast/css/text-align-last-end-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/mac/fast/css/text-align-last-end-expected.png). [image/png] [5] LayoutTests/platform/efl/fast/css/text-align-last-start-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/efl/fast/css/text-align-last-start-expected.png). [image/png] [5] LayoutTests/platform/efl/fast/css/text-align-last-left-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/efl/fast/css/text-align-last-left-expected.png). [image/png] [5] LayoutTests/platform/efl/fast/css/text-align-last-center-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/efl/fast/css/text-align-last-center-expected.png). [image/png] [5] LayoutTests/platform/mac/fast/css/text-align-last-right-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/mac/fast/css/text-align-last-right-expected.png). [image/png] [5] LayoutTests/platform/mac/fast/css/text-align-last-left-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/mac/fast/css/text-align-last-left-expected.png). [image/png] [5] LayoutTests/platform/mac/fast/css/text-align-last-start-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/mac/fast/css/text-align-last-start-expected.png). [image/png] [5] LayoutTests/platform/mac/fast/css/text-align-last-center-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/mac/fast/css/text-align-last-center-expected.png). [image/png] [5] LayoutTests/platform/efl/fast/css/text-align-last-justify-one-line-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/efl/fast/css/text-align-last-justify-one-line-expected.png). [image/png] [5] LayoutTests/platform/efl/fast/css/text-align-last-justify-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/efl/fast/css/text-align-last-justify-expected.png). [image/png] [5] LayoutTests/platform/efl/fast/css/text-align-last-right-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/efl/fast/css/text-align-last-right-expected.png). [image/png] [5] LayoutTests/platform/mac/fast/css/text-align-last-auto-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/mac/fast/css/text-align-last-auto-expected.png). [image/png] [5] LayoutTests/platform/mac/fast/css/text-align-last-justify-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/mac/fast/css/text-align-last-justify-expected.png). [image/png] [5] LayoutTests/platform/efl/fast/css/text-align-last-end-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/efl/fast/css/text-align-last-end-expected.png). [image/png] [5] LayoutTests/platform/mac/fast/css/text-align-last-justify-one-line-expected.png:0: Set the svn:mime-type property (svn propset svn:mime-type image/png LayoutTests/platform/mac/fast/css/text-align-last-justify-one-line-expected.png). [image/png] [5] Total errors found: 16 in 59 files If any of these errors are false positives, please file a bug against check-webkit-style.

I've split this patch into parsing and rendering, regarding jchaffraix's advise on irc. * parsing : https://bugs.webkit.org/show_bug.cgi?id=99439 * rendering : https://bugs.webkit.org/show_bug.cgi?id=99584

I change this bug as a meta bug of implementing the text-align-last feature, regarding the suggestion of jchaffraix on the bug, https://bugs.webkit.org/show_bug.cgi?id=99439.

As Bug 6203 (kashida justification) is moved to CSS4 (http://dev.w3.org/csswg/css-text-4/) and CSS4 currently is in draft state and it is related to text-justify and not text-align-last anyway I am removing it from here. If depended bug (parsing and rendering parts) are resolved this is also resolved AFAIK. Thank you very much for fixing this bug :)

WebKit when is it going to support it?

Every other browser supports it.

Hey guys, can this get another boost?

Why is this marked "RESOLVED FIXED"? text-align-last still doesn't work in Safari 12.0.2, STP 73, or Mobile Safari 12.1.2.

16 months after the previous comment, this still does NOT work. Can someone please change it from "RESOLVED FIXED" to something else?

(In reply to Chad von Nau from comment #42) > Why is this marked "RESOLVED FIXED"? text-align-last still doesn't work in > Safari 12.0.2, STP 73, or Mobile Safari 12.1.2. (In reply to Sander from comment #43) > 16 months after the previous comment, this still does NOT work. Can someone > please change it from "RESOLVED FIXED" to something else? As was CSS WG policy at the time of implementation, and as the spec (https://www.w3.org/TR/2013/WD-css-text-3-20131010/) said: > Prior to a specification reaching the Candidate Recommendation stage in the W3C process, all implementations of a CSS feature are considered experimental. The CSS Working Group recommends that implementations use a vendor-prefixed syntax for such features, including those in W3C Working Drafts. This avoids incompatibilities with future changes in the draft. See Bug 229083 for unprefixing it, in accordance with current spec status and CSS WG guidelines.