Closed Bug 536557 Opened 15 years ago Closed 12 years ago

Implement CSS3 text-align-last

Categories

(Core :: Layout: Text and Fonts, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: teun, Assigned: smontagu)

References

Details

(Keywords: dev-doc-complete, Whiteboard: parity-IE)

Attachments

(7 files)

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_5_8; en-us) AppleWebKit/531.21.8 (KHTML, like Gecko) Version/4.0.4 Safari/531.21.10
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.6) Gecko/20091201 Firefox/3.5.6

Please implement CSS3 text-align-last: [left, right, center, inherit, auto, justify]

Currently, there is no way in Firefox to get a single line or the last line in a paragraph to justify.

Reproducible: Always
Attached file Simple testcase
This is a simple testcase where the last line of the div should be justified as the rest.
OS: Mac OS X → All
Hardware: x86 → All
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → Trunk
Flags: in-testsuite+
Tentatively assigning to myself...
Assignee: nobody → ehsan
Flags: in-testsuite+ → in-testsuite?
Hi, Just a little comment, This feature is nearly important for Perisan Poets.
compare this Wikipedia page on IE and Firefox: http://fa.wikipedia.org/wiki/%D8%AD%D8%A7%D9%81%D8%B8#.D8.BA.D8.B2.D9.84.DB.8C.D8.A7.D8.AA (poets parts balanced on IE)
Also if you install Iranian calligraphy Font; IranNastaliq, from: http://www.scict.ir/portal/File/ShowFile.aspx?ID=bea5ca36-1fdf-41d4-8818-c1a4f9c71081 you can see how much this can make beautiful poets on this page: http://fa.wikipedia.org/wiki/%D8%B9%D9%85%D8%B1_%D8%AE%DB%8C%D8%A7%D9%85#.D8.A7.D9.86.D8.A7.D8.A8.D9.87
Ehsan, I could take this if you aren't planning to work on it in the near future.
(In reply to comment #4)
> Ehsan, I could take this if you aren't planning to work on it in the near
> future.

Please feel free, I don't think that I can work on it in the near future.  Thanks!
Attached patch PatchSplinter Review
Assignee: ehsan → smontagu
Attachment #533613 - Flags: review?(dbaron)
Attached patch testsSplinter Review
Attachment #533614 - Flags: review?(dbaron)
I don't see any reason that we should be implementing this unprefixed.  Am I missing something, or should this have a -moz-/Moz prefix?  (But for properties that are in specs, only prefix the minimum amount needed -- don't prefix internal-only constants.)

(It's also not clear to me why the spec puts first-line alignment into the existing text-align property but puts last-line alignment into its own property.  Why the inconsistency?)
In nsRuleNode, please wrap at less than 80 chars.

In property_database.js, you can test left and right as other_values
(given that neither start nor end is an initial value).

You should have big comments at the top of HorizontalAlignFrames
that the caller doesn't set isLastLine correctly in the cases
where it's not needed.

I still haven't gotten through the block/line changes, but hopefully
I'll do that on the airplane.
(In reply to comment #8)
> I don't see any reason that we should be implementing this unprefixed.  Am I
> missing something, or should this have a -moz-/Moz prefix?

I thought we weren't using the -moz prefix on things that have already been implemented unprefixed in other browsers.

> (It's also not clear to me why the spec puts first-line alignment into the
> existing text-align property but puts last-line alignment into its own
> property.  Why the inconsistency?)

My understanding from http://lists.w3.org/Archives/Public/www-style/2011Mar/0307.html is that this doing it that way is best for fallback in UAs that don't support first- and last- line alignment.
Comment on attachment 533613 [details] [diff] [review]
Patch

... continued from my previous 2 comments:

In nsBlockFrame::PlaceLine, perhaps alignLastLine should be called
isLastLine, with a comment that it's a tolerable approximation (only
accurate in the cases where we need it to be).

I wonder whether nsLineLayout really needs mTextAlign and
mTextAlignLast given that it also has mStyleText.  Getting rid of
them might make sense if it looks straightforward.

Maybe leave a comment in the code as well where you're going to
need to change things when implementing text-justify?

Anyway, r=dbaron with that.
Attachment #533613 - Flags: review?(dbaron) → review+
Comment on attachment 533614 [details] [diff] [review]
tests

It might be nice to change the tests in two ways:
 * could you expand the naming scheme from numbers to words, using
   the key that you give in the manifest?
 * could you also add tests for the single line case, where you'd
   be able to write stronger == tests (since text-align-last for
   each value of text-align should match a reference that has
   text-align set to the value of text-align-last in the test)

r=dbaron with that
Attachment #533614 - Flags: review?(dbaron) → review+
(In reply to comment #10)
> (In reply to comment #8)
> > I don't see any reason that we should be implementing this unprefixed.  Am I
> > missing something, or should this have a -moz-/Moz prefix?
> 
> I thought we weren't using the -moz prefix on things that have already been
> implemented unprefixed in other browsers.

So what browsers implement this unprefixed?  I wasn't aware of any, and I don't see any comments in the bug saying which ones do (or is comment 3 saying IE implements it?).
Yes, A screenshot from compare of http://fa.wikipedia.org/wiki/%D8%AD%D8%A7%D9%81%D8%B8#.D8.BA.D8.B2.D9.84.DB.8C.D8.A7.D8.AA (with specific font) on IE and Firefox
From: http://fa.wikipedia.org/wiki/%D9%85%D8%AF%DB%8C%D8%A7%D9%88%DB%8C%DA%A9%DB%8C:Common.css

/* To Display poems justified in IE and CSS3 (http://www.w3.org/TR/css3-text/) */
.b { /* verse container */
  text-align: justify;
  text-align-last: justify; /* for IE and CSS3 */
  border: 0;
}
(In reply to comment #13)
> So what browsers implement this unprefixed?  I wasn't aware of any, and I
> don't see any comments in the bug saying which ones do (or is comment 3
> saying IE implements it?).

IE does
http://msdn.microsoft.com/en-us/library/ms531163(v=vs.85).aspx
but a prefix is needed for IE (8+) strict mode, according to that page
It appears from my testing that IE only supports text-align-last when text-align is set to justify. Chrome, Konqueror, Opera and Safari don't appear to support it at all, either prefixed or unprefixed.

(In reply to comment #15)
> IE does
> http://msdn.microsoft.com/en-us/library/ms531163(v=vs.85).aspx
> but a prefix is needed for IE (8+) strict mode, according to that page

What the page says is "Windows Internet Explorer 8. The -ms-text-align-last attribute is an extension to CSS, and can be used as a synonym for text-align-last in IE8 Standards mode." I understand that to mean that it works unprefixed in either Standards or Quirks mode, but the prefixed form only works in Standards mode. That sounds a little illogical to me, but testing in IE9 confirms that that is indeed the behaviour. 

So what's the conclusion? Does "r=dbaron with that" include changing to -moz-text-align-last, or can I check it in unprefixed so that pages like the fa.wikipedia.org examples will work in Gecko?
Whiteboard: parity-IE
Does IE9 still support it unprefixed (in their standards mode)?
(In reply to comment #17)
> Does IE9 still support it unprefixed (in their standards mode)?

yes, at least for text-align-last: justify and LTR text; see screenshot [*]
Testcase:
http://dev.l-c-n.com/CSS3/text-align-last2b.html

(it works for both LTR and RTL with text-align-last:center)

[*] I used this service to take the screenshot:
http://ipinfo.info/netrenderer/index.php
Also, they use it on their IE9+ demonstration site: http://ie.microsoft.com/testdrive/Performance/TextJustificationAnimated/Default.html
I think if the only other implementation is IE, we should implement this prefixed for now. We can drop the prefix once the spec hits CR.

Also, dbaron mentioned that the IE implementation only uses text-align-last when justification is turned on. Do we prefer that behavior, or honoring it always? Why?
I thought you were the one who said that; I don't have information one way or the other.
No, I was the one who said that (in comment 16 above). I don't know how much of a use case there is for supporting every single combination of text-align and text-align-last, but I've certainly seen poetry set in verses with the equivalent of "text-align: start; text-align-last: end;"
(In reply to comment #22)
> ... but I've certainly seen poetry set in verses with the
> equivalent of "text-align: start; text-align-last: end;"

The Flemish Dadaist poet Paul Van Ostayen did quite a few things like that (a google image search doesn't turn the one I am thinking about, though, but has other examples).
Was that "text-align: start; text-align-last" or "text-align: start end"?
That's not actually the last line in the same sense as text-align-last. That's the last semantic line; if it wraps to two lines, then both lines will be right-aligned. So you can't use text-align-last for that.
Hi. Can we use this on Wikipedia now? :) (or -moz- CSS prefix needed?)
(In reply to comment #27)
> Hi. Can we use this on Wikipedia now? :) (or -moz- CSS prefix needed?)

No, I don't think the patch has landed yet.
What's blocking this bug?
Checked in (with -moz- prefix):

https://hg.mozilla.org/integration/mozilla-inbound/rev/9217303c2e5c
https://hg.mozilla.org/integration/mozilla-inbound/rev/6605cc311ec5
Flags: in-testsuite? → in-testsuite+
Target Milestone: --- → mozilla11
Target Milestone: mozilla11 → mozilla12
Backed out of inbound for M4 failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=6605cc311ec5
https://tbpl.mozilla.org/php/getParsedLog.php?id=8507781&tree=Mozilla-Inbound
{
7639 ERROR TEST-UNEXPECTED-FAIL | /tests/layout/style/test/test_bug657143.html | Experimental -moz- CSS property list should be alphabetical and not contain mature properties - got "-moz-text-align-last,text-decoration,text-indent,text-overflow,text-shadow,text-transform,top,unicode-bidi,vertical-align,visibility,white-space,width,word-spacing,word-wrap,z-index,-moz-animation-delay,-moz-animation-direction,-moz-animation-duration,-moz-animation-fill-mode,-moz-animation-iteration-count,-moz-animation-name,-moz-animation-play-state,-moz-animation-timing-function,-moz-appearance,-moz-backface-visibility,-moz-background-inline-policy,-moz-binding,-moz-border-bottom-colors,-moz-border-image-outset,-moz-border-image-repeat,-moz-border-image-slice,-moz-border-image-source,-moz-border-image-width,-moz-border-left-colors,-moz-border-right-colors,-moz-border-top-colors,-moz-box-align,-moz-box-direction,-moz-box-flex,-moz-box-ordinal-group,-moz-box-orient,-moz-box-pack,-moz-box-sizing,-moz-column-count,-moz-column-fill,-moz-column-gap,-moz-column-rule-color,-moz-column-rule-style,-moz-column-rule-width,-moz-column-width,-moz-float-edge,-moz-font-feature-settings,-moz-font-language-override,-moz-force-broken-image-icon,-moz-hyphens,-moz-image-region,-moz-orient,-moz-outline-radius-bottomleft,-moz-outline-radius-bottomright,-moz-outline-radius-topleft,-moz-outline-radius-topright,-moz-perspective,-moz-perspective-origin,-moz-stack-sizing,-moz-tab-size,-moz-text-blink,-moz-text-decoration-color,-moz-text-decoration-line,-moz-text-decoration-style,-moz-text-size-adjust,-moz-transform,-moz-transform-origin,-moz-transform-style,-moz-transition-delay,-moz-transition-duration,-moz-transition-property,-moz-transition-timing-function,-moz-user-focus,-moz-user-input,-moz-user-modify,-moz-user-select,-moz-window-shadow", expected "-moz-animation-delay,-moz-animation-direction,-moz-animation-duration,-moz-animation-fill-mode,-moz-animation-iteration-count,-moz-animation-name,-moz-animation-play-state,-moz-animation-timing-function,-moz-appearance,-moz-backface-visibility,-moz-background-inline-policy,-moz-binding,-moz-border-bottom-colors,-moz-border-image-outset,-moz-border-image-repeat,-moz-border-image-slice,-moz-border-image-source,-moz-border-image-width,-moz-border-left-colors,-moz-border-right-colors,-moz-border-top-colors,-moz-box-align,-moz-box-direction,-moz-box-flex,-moz-box-ordinal-group,-moz-box-orient,-moz-box-pack,-moz-box-sizing,-moz-column-count,-moz-column-fill,-moz-column-gap,-moz-column-rule-color,-moz-column-rule-style,-moz-column-rule-width,-moz-column-width,-moz-float-edge,-moz-font-feature-settings,-moz-font-language-override,-moz-force-broken-image-icon,-moz-hyphens,-moz-image-region,-moz-orient,-moz-outline-radius-bottomleft,-moz-outline-radius-bottomright,-moz-outline-radius-topleft,-moz-outline-radius-topright,-moz-perspective,-moz-perspective-origin,-moz-stack-sizing,-moz-tab-size,-moz-text-align-last,-moz-text-blink,-moz-text-decoration-color,-moz-text-decoration-line,-moz-text-decoration-style,-moz-text-size-adjust,-moz-transform,-moz-transform-origin,-moz-transform-style,-moz-transition-delay,-moz-transition-duration,-moz-transition-property,-moz-transition-timing-function,-moz-user-focus,-moz-user-input,-moz-user-modify,-moz-user-select,-moz-window-shadow,text-decoration,text-indent,text-overflow,text-shadow,text-transform,top,unicode-bidi,vertical-align,visibility,white-space,width,word-spacing,word-wrap,z-index"
}

https://hg.mozilla.org/integration/mozilla-inbound/rev/a0c02bb5fbb4
Status: NEW → ASSIGNED
Flags: in-testsuite+
Target Milestone: mozilla12 → ---
Agggh! I saw that failure on try and fixed it, but not in the tree I then checked in from!
@Simon Montagu:
It is great if you can test Firefox with this patch on http://fa.wikipedia.org/wiki/%D8%AD%D8%A7%D9%81%D8%B8#.D8.BA.D8.B2.D9.84.DB.8C.D8.A7.D8.AA , because we added -moz-text-align-last in our CSS (also please install this font http://www.scict.ir/portal/File/ShowFile.aspx?ID=bea5ca36-1fdf-41d4-8818-c1a4f9c71081 before testing) Expected result is attached (https://bugzilla.mozilla.org/attachment.cgi?id=534295 )
Thanks :)
Thanks a lot :)
https://hg.mozilla.org/mozilla-central/rev/69eb27cd5313
https://hg.mozilla.org/mozilla-central/rev/aa62396ae6bd
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
Depends on: 726392
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: