We don't support justified text yet. Does this fit in with the whitespace work? It's also a style issue. See URL for test case and sample code.

It is now partially implemented, though it could use some work. I'm leaving the bug open, but at a reduced priority because there are some easy to create conditions where the justification fails to operate as expected.

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

Setting all current Open/Normal to M4.

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

The regression I reported in 3222 occurred between the builds of 99-02-11 and 99-02-16.

See also http://www.angelfire.com/ms/dhtml (bug #2475)

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

See also http://rucs.rutgers.edu/emailtf.wp1.html (bug #1783)

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

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

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

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

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

See also http://schist/4447.html

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

Increasing priority a little because this is a CSS1 issue and there are some CSS2 issues at enhancement/P5. The interaction of text-align:justify and white-space:pre is covered by bug 6011.

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

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

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

A simple testcase (also demonstrating bug 20260) is at http://www.thirdnipple.com/mozilla/. This also occurs without the table in this page, but I didn't feel like typing a whole screen's worth of filler text :) I'd suggest that this is a problem which ought to be fixed before beta, as a lot of websites rely on full justification to look nice... The priority IMHO ought to be higher, but I'll leave that to the people who know this bug better.

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

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

Updating to default Style System Assignee...kipp no longer with us :-(

Migrating from {css1} to css1 keyword. The {css1}, {css2}, {css3} and {css-moz} radars should now be considered deprecated in favour of keywords. I am *really* sorry about the spam...

Assigned to kipp so that this hopefully doesn't get overlooked for much longer by the new layout kings.

based on the number of pages effected, the comments in this bug, and follow-up comments on the newsgroup, maing this P1.

Setting the keyword all open [4.xp] bugs to 4xp.

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

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

Adding 'beta1' to keywords. Beta1 should support all CSS1 styles.

Putting on PDT+ radar for beta1.

this is likely to be very hard.

This used to work (somewhat buggily) about a year ago, so the code is there, somewhere. It stopped working correctly in March 1999 and then I think Kipp disabled it. It may also have had i18n problems.

Give it to Knuth! Knuth will code anything!

I looked at this today. There is some code in the block frame that figures out whether a line can be fully justified and passes that boolean to nsLineLayout::HorizontalAlignFrames(). This code was ifdefed out and I re-enabled it in my build. Even with this enabled, though, text does not get fully justified because HorizontalAlignFrames() doesn't do anything for that case. I'm attaching a really simple test case which has a single line of fully justified text. Since I can't find any code that tries to fully justify text, implementing it will take some time. This is definitely not possible for M14. Updating status whiteboard to indicate as such.

Here is Kipp's initial justify work: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=kipp*&whotype=regexp&sortby=Date&hours=2&date=explicit&mindate=10%2F25%2F1998+00%3A00%3A00&maxdate=10%2F27%2F1998+09%3A00%3A00&cvsroot=%2Fcvsroot It seemed to involve calculating a special wordspacing, and adding the extra space to the whitespace glyph width. There was also a JustifyFrames method that took care of this, but, that was probably replaced by nsInlineFrame::AdjustFrameSize

A good place to start would probably be enabling the disabled code in nsBlockFrame::PlaceLine, and seeing what happens.

Removing PDT+ and beta1 keyword, because this work cannot be done for M14. We also think it's not required for beta 1 that we support full justification

mine! mine mine mine! all mine! whoo-hoo!

Note that the justification algorithm should handle the properities word-spacing, letter-spacing, and white-space (and probably others...).

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

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

I've done some work on this, and I'm ready to try testing it. However, I don't have a compiler. If anyone wants to take a look, I've uploaded a copy here: http://fantasai.tripod.com/Mozilla/TXJ/ So, if anyone gets this to compile, please tell me what happens? I'm rather curious to know what it will do.

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

moving all buster m15 bugs to m16.

I have implemented this. I will attach the patch momentarily. I got a few ideas from fantasai's patch (thanks!) but mine is rather different. In particular, I've avoided adding any fields to nsTextFrame or any of the style contexts. The basic idea is that nsLineLayout collects information from the text frames during reflow; in particular it collects the number of spaces in each text frame. It then uses this in HorizontalAlignFrames to distribute leftover line space appropriately to each text frame. Justified text frames notice when they are bigger than they need to be and distribute the extra space internally in the correct manner. There are probably some bugs. In fact, I found and fixed a number of other bugs while I was doing this; they are marked "NOTE:" in the patch. Regarding PRE: I treat nonbreaking spaces (which is what all PRE spaces turn into) as regular spaces for the purposes of justification. Spaces derived from tab expansion are also treated the same way. I don't know if this is "correct" or not, but it works in that it makes justification useful in the context of PRE. Word-spacing and letter-spacing work. It all even works in Composer (modulo a few display glitches).

I have to say I don't think justification should do anything for PRE, since the lines in PRE are broken by forced breaks, and forced breaks are not justified.

Turning off justification for preformatted text is as simple as making mJustifying depend on !mPreformatted, a one-line change to TextStyle::TextStyle(). Then such text frames will not report any spaces or letters to nsLineLayout, and will receive no extra space for justification. One other thing I should mention --- single-word lines are currently not justified in any way. My patch prepares the way for using letter-spacing to stretch such words across the line, but I haven't actually put it in, because I think it looks ugly.

You're quite welcome! I'm glad it helped, at least. Can't really expect one's first C++ code to be perfect. ^o^ Actually, I still don't even know if mine works or not. -_-;; I thought of giving Line Layout the number of spaces and characters during reflow, but I figured, why waste function calls when most text isn't justified anyway? Maybe that's an invalid argument. I wouldn't know. --Preformatted text can be justified if it's wrapped (-moz-pre-wrap). --If the author wishes to suppress character-spacing justification, they can simply specify 'letter-spacing: 0'. According to the CSS spec, the UA may only adjust the letter-spacing if it is set to 'normal' (the default). --Single word lines (when 'letter-spacing' is set even if extending character-spacing is implemented) and lines without text need to be right justified if the text direction is rtl. Oh, and you really should take up word-spacing issues in bug 1046. :) um.. Did the priority change or is it just my imagination?

> I thought of giving Line Layout the number of spaces and characters during > reflow, but I figured, why waste function calls when most text isn't justified > anyway? The impact of that is minimal. There should be basically no impact on the performance of laying out non-justified text. Laying out justified text is slower than normal text. It could be sped up a little by caching the value of each text frame's "justifcation extra space" somewhere, but that would require significant additional space (if you put it in nsTextFrame) or complexity (if you put it in an extension record in nsLineBox) --- not worth it in my opinion. > --Preformatted text can be justified if it's wrapped (-moz-pre-wrap). Is there a consensus on whether this should be justified? What's it used for? It's easy to tweak the patch to handle these cases in whichever way is desired. RTL support is a good point. It's probably broken. How can I test that? There seem to be a lot of lurking RTL problems in this area, I'd rather not be the one to have to fix them all. Browsing around, I have noticed that some sites look bad because the existing ShouldJustifyLine code is inadequate. E.g. <p>Hello kitty<br> gets justified. We can address that issue separately. This bug has been P1 for a while, which is why I'm here.

-moz-pre-wrap is used for <PRE width="x">. It keeps whitespace intact, but allows wrapping. I don't see any reason why it shouldn't be justified, if full justification was specified on it. As for RTL.. your ApplyFrameJustification returns an nscoord value. Subtract this from remainingWidth and delete the 'else' before the RTL check. I did the same thing in mine--see the code under "// Apply Justification" in txj.txt. I didn't see any record for a priority change, so I was wondering..

If you're going to allow justify on PRE elements, I think you should probably add a rule to ua.css so that PRE can't inherit 'text-align: justify', but it would have to be specified explicitly. I think generally justify goes against the meaning of PRE.

I have updated the patch. It fixes the bug with BR-terminated lines; they are no longer justified. I have disabled justification for PRE and -moz-pre-wrap text; you can easily see where to turn this on again if desired. I have also brought the patch up to date with the tip. I also fixed an existing bug in nsTextFrame in the "MeasureTextRuns" code for Win32, where some lines were not being correctly wrapped. All the examples I've looked at, including all the examples from the duplicates of this bug, are looking good. I tried some RTL tests, but the existing RTL handling already has bugs and it's hard to tell whether I've introduced any new ones. I decided not to justify -moz-pre-wrapped. The reason is that trailing whitespace is not stripped from -moz-pre-wrapped text, so wrapped lines almost always have a trailing space. This means that justification will not produce a straight right margin. You could argue for stripping trailing whitespace from -moz-pre-wrapped text, but that's a separate issue.

Robert: On first review, your new patch looks great. I'll spend some time reviewing it in greater detail this afternoon. If all looks good, I'll check it in early tomorrow morning. Or, would you rather have the honors? Makes no difference to me. Do you have cvs access rights? If I do the checkin, it's slightly easier for me because I can merge your changes with my own for 12+ other bugs I've been holding and get it all in at once. Many thanks to fantasia for the first cut, and as always to david for his insights.

I don't have CVS access, so you'll have to check it in.

I've done a thorough read-through, and it all looks great. The only additions I plan on making are a few comments, a few assertions (checking for null pointers passed into new methods), and some cosmetic changes to bring it up to our coding standard (method arguments take the form "aArgName".) In a second pass, it would be better if the justification-specific data items (mTextJustificationNumSpaces, etc.) were off in their own struct, and that struct was dynamically allocated only when needed. This would save us 16 bytes per PerFrameData object in the case where justification isn't needed. We can look into that as an optimization later. similarly, I've already moved all the booleans in that data structure into a single PRInt32 bitfield, so I'll just go ahead and do the same for mIsBullet now. I'll run some tests, and if it all is well and if the tree is in good shape tomorrow early am PST, it'll be in tomorrow's build.

> method arguments take the form "aArgName" Oops, thanks for reminding me. I didn't realize the size of PerFrameData was important. They only exist while their line is being reflowed, right? Actually, those fields are costing 8 bytes per PerFrameData, and 8 bytes per nsLineLayout. (So having a dynamic structure within PerFrameData would only save 4 bytes for the non-justifying case.) It probably wouldn't be too hard to get to 0 bytes per PerFrameData and 16 bytes per nsLineLayout, by getting rid of ComputeJustificationWeights and keeping running totals of the total space and letter counts in nsLineLayout::PlaceFrame. Alternatively, I could get rid of all the space overhead if I could just call a method in nsTextFrame to fetch the space and letter counts during ComputeJustificationWeights, but I'm not sure how to do the cast in a totally safe way.

At some point, now that nsIFrame::AdjustFrameSize isn't going to be used, it should be deleted. For optimization purposes, maybe we could add a method to nsIFrame that retrieves space and letter counts. The default implementation would return 0s, and nsTextFrame would override with the snippet that I added to nsTextFrame::Reflow. This would be called during the ComputeJustificationWeights traversal, and nothing would need to be recorded in the PerFrameData --- zero space overhead. It would also get rid of the need to correct overestimated space counts after whitespae trimming, because whitespace would already be trimmed by the time we compute the counts. I'll try this out once the basic patch is in.

Most tests passed perfectly. But I had to make a small change to your logic to get mail to run correctly. I don't fully understand this, because the way you had the code looked fine. Basically, I had to revert a change you made, as shown below. There are two adjacent code snippets marked with comments starting with O'Callahan XXX: for (lastSegment = 0; textRun.mBreaks[lastSegment] < numCharsFit; lastSegment++) ; NS_ASSERTION(lastSegment < textRun.mNumSegments, "failed to find segment"); // now we have textRun.mBreaks[lastSegment] >= numCharsFit /* O'Callahan XXX: This snippet together with the snippet below prevents mail from loading Justification seems to work just fine without these changes. We get into trouble in a case where lastSegment gets set to -1 if (textRun.mBreaks[lastSegment] > numCharsFit) { // NOTE: this segment did not actually fit! lastSegment--; } */ } /* O'Callahan XXX: This snippet together with the snippet above prevents mail from loading if (lastSegment < 0) { // no segments fit break; } else */ if (lastSegment == 0) { // Only one segment fit prevColumn = column; prevOffset = aTextData.mOffset; } else { I plan on checking it in this way because it passes all my tests, plus the justification works fine. Maybe you could spend a little time figuring out why your code creates some bad state that triggers an assertion in nsBlockFrame: NS_ASSERTION(aState.IsImpactedByFloater(), "redo line on totally empty line");

OK. I made that change to resolve a rather obscure bug that I believe is exposed by (rather than caused by) justification. Assuming I can reproduce it after justification lands, I will file a separate bug and try to resolve the situation. The bug involves some lines not being broken correctly. Basically, the Win32 GetWidth using textRun will occasionally return a numCharsFit that is in the middle of a word. MeasureText simply can't handle that; it computes a value for "lastSegment" where the end of the line actually falls somewhere inside the segment, and then assumes that all the text in lastSegment will actually fit on the line, which is wrong. My changes simply try to do the right thing in this case.

fix checked in for Robert O'Callahan <roc+moz@cs.cmu.edu>. Should be able to verify against 4/17/00 build.

Looks good in 2000041708. I can't reproduce the GetWidth-related bug I was seeing. Let's forget about it for now :-). I will file a bug regarding the excessive space consumption, and provide a patch when I get around to it. I will also file a bug regarding the minor display glitches in Composer. It is probably a layout problem and might come up in non-Composer contexts (dynamic content).

Tested on Windows, Mac and Linux. Windows and Linux work fine but justification on Mac is buggy. Separate bug written up for that. Verifying this bug fixed.