For inline containers that are split to multiple frames (because they contain bidi text, line breaks, or other inlines), right paddings, borders, and margins, are applied to the first (logical) frame, and left paddings, borders, and margins are applied to the last logical frame. This is incorrect for the RTL and bidi cases. In RTL context, at the minimum "first" and "last" should be reversed. The CSS 2.1 standard, section 8.6 (http://www.w3.org/TR/CSS21/box.html#q11) goes further: >For each line box, UAs must take the inline boxes generated for each element and render the margins, borders and padding in visual order (not logical order). >When the element's 'direction' property is 'ltr', the left-most generated box of the first line box in which the element appears has the left margin, left border and left padding, and the right-most generated box of the last line box in which the element appears has the right padding, right border and right margin. >When the element's 'direction' property is 'rtl', the right-most generated box of the first line box in which the element appears has the right padding, right border and right margin, and the left-most generated box of the last line box in which the element appears has the left margin, left border and left padding. Note that bug 299063 is a special case of this bug. Also note that margins on inlines in bidi text are currently not rendered at all, due to bug 121633.

Testcase for left and right padding, border, and margin in (a) bidi text and (b) text in RTL context. Paddings and borders appear in obviously wrong places in all of the examples. Margins don't appear at all.

Ignore the <title> on that testcase - I was using a different testcase as a template and forgot to update the title.

Ignore the <title> on that testcase - I was using a different testcase as a template and forgot to update the title.

I think there are following choices to fix the problem: a) We can do something like previous implementation of reordering. I mean extracting the leaf frames and repositioning them to they correct place, and then repositiong their container frames. (I can prepare a new patch if you are interested in this solution) b) We can add a new linked list like Get{Prev,Next}BidiFrame(?) to splitted frames to represents their visual order. nsBidiPresUtils::ReorderFrames() should set them. Then the statments like following should work with new linked list. (nsInlineFrame::ReflowFrames) aMetrics.width = size.width; if (nsnull == GetPrevContinuation()) { aMetrics.width += aReflowState.mComputedBorderPadding.left; } if (NS_FRAME_IS_COMPLETE(aStatus) && (!GetNextContinuation() || GetNextInFlow())) { aMetrics.width += aReflowState.mComputedBorderPadding.right; }

This patch fixes margins, and paddings. But borders still need a little more work. nsInlineFrame::GetSkipSides() must be tuned to use Get{Next,Prev}VisualContinuation() instead. Also reverse iterating child frames is now O(n^2) which must be fixed in next version.

I don't know this code (specifically line layout and the bidi code) nearly well enough to review it. That said, adding the two members to nsIFrame is almost certainly undesirable. And the GetPrevSiblingFor() for method raises O(N^2) alarms for me. What makes this algorithm not O(N^2)?

About Get{Next,Prev}VisualContinuation, I could store those pointer locally in nsBidiPresUtil, but I think other classes may need to know the visual order of continuation frames too, for example in caret movment or in nsInlineFrame::GetSkipSides() or ... (?) Instead of GetPrevSiblingFor(), It's possible to store the list locally (in an array for example) and iterate the array in reverse order. It would be O(n). bz who can I ask for a review then?

Well, first of all are those pointers needed long term? Or do you just need them temporarily (eg until reflow finishes)? For example, GetSkipSides() doesn't need to know the next/prev visual, just whether there is one; we should be able to use frame state bits for that, if there are free ones. As for review... Uri probably knows the bidi stuff as well as anyone at this point. For line layout, dbaron or roc I'd guess.

I'm not sure but i think it would really help in caret movement too. In the case of reflow it's enough too know if the frame is first(or last) in visual order or not. So state bits will suffice.

Comment on attachment 213317 [details] [diff] [review] V1 In addition to the memory and performance problems pointed out by bz, there's a functional problem with this patch: It operates on a line-per-line basis only, so that (e.g.) the right padding is applied to the rightmost frame *of each line*. This is wrong, as right padding/border/margin should only be applied to the rightmost frame on the first line in RTL, and the rightmost frame of the last line in LTR (the reverse is true, of course, for left margin/padding/border). The patch also doesn't include the necessary changes to GetSkipSides, so that the actual borders are still drawn in the wrong place. Also, the case of a left margin/border/padding applied to a frame on the second line of RTL text isn't handled properly - the styled text is pushed to the right, instead of the following text being pushed to the left. I'll attach a screenshot of my testcase with the patch applied, which should demonstrate what I said above.

Screenshot of the testcase with patch V1 applied, and the problems pointed out.

I prepared a new patch. It fixes the multi-line problem. Paddings is also considered, so 299063 is fixed as well. For the performance issue commented by bz, I used a 1 byte state value instead of those two pointers.(8 bytes!)

To use this testcase, adjust the size of the window so that the lines will break between the Persian and Hebrew word on each line. The left paddings and borders should always be on the first line (i.e., on the Persian word), and the right paddings and borders should be on the second line (Hebrew word). Not that this works correctly on the current trunk, but is regressed by patch V2: the borders and paddings appear in the wrong places.

I solved the problem with wrapped lines. Now I use this simple principle that, if for some frame on the line 'frame->GetPrevContinuation() == nsnull' is true, then it's the first line that the frame or one of its continuations appear. And likewise if 'frame->GetNextContinuation() == nsnull' is true, then it's the last line that the frame or one of its continuations appear.

An extended testcase, which includes ltr and rtl wrapped lines.

Comment on attachment 220385 [details] [diff] [review] V3 Overall, this looks good (although in the long term, I think we should aim for a solution where the frames are positioned correctly to begin with, instead of being positioned LTR and then repositioned). I only reviewed the nsBidiPresUtils part of the patch. As Boris wrote in comment #8, you will still need a review from dbaron or roc for the other parts. My comments are below: >Index: layout/base/nsBidiPresUtils.cpp >=================================================================== >+ isLeftMost = (isLTR && endingState & CONTENT_FIRST_LINE) Please add parentheses around |endingState & CONTENT_FIRST_LINE|, for readability (also on similar lines below). The logic in PositionChildFrames() is not trivial, so adding a few comments can help (specifically: explain the logic behind the way you compute isLeftMost/isRightMost; explain why you are clearing the endingState flags (when you do); explain why you are decrementing the count in mContentToFrameIndex.) You can do all of this in one comment, or in separate comments. >+ while (frame) { >+ PositionChildFrames(frame, isOddLevel, x); >+ frame = isOddLevel ? GetPrevSiblingFor(aFrame, frame) : frame->GetNextSibling(); >+ } This is O(n^2). Perhaps instead you can use a local nsVoidArray, populate it with the children, and then traverse it backwards? >+ if (aFrame->GetPrevContinuation() == nsnull) Change to |if (!aFrame->GetPrevContinuation())|, please (also two lines down). You are using mContentToFrameIndex in a way that is quite different than its original use, which is confusing. (specifically, the values you store in it are counts, not indexes as the name implies). I suggest using a separate (appropreately-named) hash for this. If you can have this as a local variable (passed as a parameter to PositionChildFrames()), instead of a memeber, I think it would be even better. >Index: layout/base/nsBidiPresUtils.h >=================================================================== >+ void PositionChildFrames(nsIFrame* aFrame, PRBool isOddLevel, int &left); >+ >+ void SetContentsEndings(nsIFrame *aFrame); Please document these methods. I suggest renaming PositionChildFrames() to RepositionFrame(), because it positions the frame itself (and its children), not just the children. |left| should be nscoord, not int. Also, please use |nscoord& left|, not |nscoord &left| (in the cpp file, too).

One more thing: >+ void PositionChildFrames(nsIFrame* aFrame, PRBool isOddLevel, int &left); Please prefix all argument names with "a": void PositionChildFrames(nsIFrame* aFrame, PRBool aIsOddLevel, nscoord& aLeft);

Uri, Many thanks for you precise review. I applied you comments on the patch.

Comment on attachment 221062 [details] [diff] [review] V4 Nice. Just a few more nits: >Index: base/nsBidiPresUtils.cpp >=================================================================== >+nsBidiPresUtils::SetLeftAndRightMost(nsIFrame* aFrame, >+ nsLineContentsEndings* aContentsEndings, >+ nsLineContentsCounts* aContentsCounts, >+ PRBool& aIsLeftMost /* out */, >+ PRBool& aIsRightMost /* out */) const >+{ >+ aFrame->SetBidiLeftMost(aIsLeftMost); >+ aFrame->SetBidiRightMost(aIsRightMost); >+} I would suggest renaming this method to IsLeftOrRightMost, and doing the actual setting (i.e. the calls to SetBidi...Most) outside it (in RepositionFrame, right after calling it). >+ nsMargin margin; This line is not indented correctly (nneds to move one space to the right). >Index: base/nsBidiPresUtils.h >=================================================================== >+ * To determine if it's the first or the last appearance of a content on the >+ * line, we use aFrame->GetPrev/NextContinuation(). >+ * If for a frame 'aFrame->GetPrevContinuation()==nsnull' be true, then it's >+ * the first line which it's content appears on. >+ * And likewise if for a frame 'aFrame->GetNextContinuation()==nsnull' be true, >+ * then it's the last line which it's content appears on. This part of the comment describes the implementation, so I suggest moving it to the cpp file. Also: - on lines 3,5: "be true" => "is true" - on lines 4,6: "it's content" => "its content" >+ * Determine if aFrame is leftmost or rightmost, and Set aIsLeftMost and >+ * aIsRightMost values. >+ * >+ * A frame is leftmost if it's the first appearance of its content on the line >+ * and the content is on it's first line if it's LTR or the content is on it's >+ * last Line if it's RTL. >+ * A frame is rightmost if it's the last appearance of its content on the line >+ * and the content is on it's first line if it's RTL or the content is on it's >+ * last Line if it's LTR. - on line 1: "Set" => "set" - on lines 5,8: "it's first line" => "its first line" >+ * Since we lie frames down from left to right (in both LTR and RTL) visiting a >+ * content for the first time, means it's the first appearance of that content >+ * on the line. >+ * To determine if it's the last appearance of a content on line or not, we >+ * count the number of frames of the content on the line, and then reduce >+ * it when we lied down a frame of the content. If this value become 1 it >+ * means that it's the last appearance of the content on this line. Again, this part describes implementation, so it belongs in the cpp file. - on line 1: "lie frames down" => "lay out frames"; add a comma (,) before "visiting". - on line 6: "lied down" => "lay out"; "become" => becomes. Please do document here (in the h file) the parameters for SetLeftAndRightMost, like you did for SetContentsEndingsAndCounts. r+ (on the nsBidiPresUtils part only!), with the corrections above made.

Thanks again. Sorry for my weak English! ;)

Comment on attachment 221272 [details] [diff] [review] V5 r=me for the nsBidiPresUtils part. I'm requesting addl. review from dbaron to remind us that the other parts still need review.

We really need to try to fix this without adding any fields to nsIFrame.

Why don't we call ReorderFrames earlier in nsBlockFrame::PlaceLine, before HorizontalAlignFrames gets called? Then we wouldn't need HorizontalAlignReorderedFrames. The state bits indicating whether an inline frame is visually left-most or visually right-most can be placed in nsInlineFrame's regular frame state bits (see nsInlineFrame.h). We don't need IsLeftMost/IsRightMost in nsIFrame; I think HorizontalAlignFrames could just call GetSkipSides to check if a margin should be skipped. I think the content endings and counts stuff could be improved. I think going on content isn't a great idea in case we have situations where we have wrapper frames for inline content (I don't think we do that now, but we might want to). A better way would be to have a single hashmap from frames F in this line to pairs of nsIFrame* mFirstVisualFrame (the first visual frame in the continuation chain containing F, or nsnull) and PRUint32 mFrameCount (the number of frames in the continuation chain containing F, if F is the first visual frame). Initialize this map to (null, 0) for all frames in the line by traversing the frames in the line once. Then during repositioning, as we visit each frame F in visual order, if mFirstVisualFrame is null then we know that this is the first visual frame; we can then follow the NextContinuation and PrevContinuation chains to count all continuations in this line, and set their mFirstVisualFrames to point to F. As we visit each frame in the chain we can decrement the count stored in the first visual frame's hash entry, and detect when we're visiting the last frame.

(In reply to comment #23) > Why don't we call ReorderFrames earlier in nsBlockFrame::PlaceLine, before > HorizontalAlignFrames gets called? Then we wouldn't need > HorizontalAlignReorderedFrames. We cannot call HorizontalAlignFrames() after ReorderFrames(), since ReorderFrames() does not update per span and per frame data. (psd and pfd) and HorizontalAlignFrames() uses psd and pfds. In fact there is no need to call HorizontalAlignReorderedFrames() since the line width shouldn't change after reordering. I used this as a workaround for a bug in nsInlineFrame which adding border and padding is not direction aware! (http://lxr.mozilla.org/mozilla/source/layout/generic/nsInlineFrame.cpp#442) I will remove HorizontalAlignReorderedFrames() in next patch. > The state bits ... can be placed in nsInlineFrame's regular frame state bits ... OK, I will move it to nsInlineFrame. > A better way would be to have a single hashmap from frames F in this line... This way has performance problem. Consider a huge text with N words, and each line places W words on it. Then we would have N / W continuations and N / W lines. For each line we are traversing about N / W continuations. So overall complexity would be O(N^2 / W^2). The text may be BiDi which will produce additional continuations too. > I think the content endings and counts stuff could be improved. I think going > on content isn't a great idea in case we have situations where we have wrapper > frames for inline content (I don't think we do that now, but we might want to). I think this way is efficient and clean. I relied on http://wiki.mozilla.org/Gecko:Continuation_Model which says: "Certain kinds of frames create multiple child frames for the _same_ content element: * nsSimplePageSequence creates multiple page children, each one associated with the entire document, separated by page breaks * nsColumnSetFrame creates multiple block children, each one associated with the column element, separated by column breaks * nsBlockFrame creates multiple inline children, each one associated with the same inline element, separated by line breaks, or by changes in text direction"

(In reply to comment #24) > We cannot call HorizontalAlignFrames() after ReorderFrames(), since > ReorderFrames() does not update per span and per frame data. (psd and pfd) > and HorizontalAlignFrames() uses psd and pfds. OK > In fact there is no need to call HorizontalAlignReorderedFrames() since the > line width shouldn't change after reordering. I used this as a workaround for > a bug in nsInlineFrame which adding border and padding is not direction > aware! > (http://lxr.mozilla.org/mozilla/source/layout/generic/nsInlineFrame.cpp#442) > I will remove HorizontalAlignReorderedFrames() in next patch. Okay. So nsInlineFrame adds left border and padding to the first (content-order) frame on the line. Can't you fix that when you reposition frames, by removing the left border and padding from the first content-order frame and adding it to the correct frame? Or maybe you already do... > > A better way would be to have a single hashmap from frames F in this line... > > This way has performance problem. Consider a huge text with N words, and each > line places W words on it. Then we would have N / W continuations and N / W > lines. For each line we are traversing about N / W continuations. So overall > complexity would be O(N^2 / W^2). The text may be BiDi which will produce > additional continuations too. I don't follow this example. Using my proposed scheme, if there are N frames in the line and in the worst case they're all in the same continuation chain, it takes O(N) time to set up the hash map, then when we process the first visual frame, we spend O(N) time scanning the continuation list. Then processing all the other frames takes just O(1) time each. So overall it's O(N), asymptotically optimal. > I think this way is efficient and clean. I relied on > http://wiki.mozilla.org/Gecko:Continuation_Model which says: > > "Certain kinds of frames create multiple child frames for the _same_ > content element: > > * nsSimplePageSequence creates multiple page children, each one > associated with the entire document, separated by page breaks > * nsColumnSetFrame creates multiple block children, each one associated > with the column element, separated by column breaks > * nsBlockFrame creates multiple inline children, each one associated > with the same inline element, separated by line breaks, or by changes in text > direction" That's true but I'm talking about something different. That page talks about situations where sibling frames have the same content. I'm talking about situations where parent and child frames have the same content. An example is if you have a block that's "overflow:auto". We create an nsHTMLScrollFrame whose child is an nsBlockFrame. Both of these frames have the same mContent. As far as I know we don't currently do anything like this for nsInlineFrame, but it's easy to imagine that we might want to, so I'd like to choose a solution that doesn't break if we do.

> As far as I know we don't currently do anything like this for nsInlineFrame, We do it sorta the other way around. For example, if using ::before or ::after, you'll get nested inline frames with the same mContent -- one for the actual inline, one for the anonymous content.

> Okay. So nsInlineFrame adds left border and padding to the first > (content-order) frame on the line. Can't you fix that when you reposition > frames, by removing the left border and padding from the first content-order > frame and adding it to the correct frame? Or maybe you already do... No! Since the correnct frame can be on another line, then we will have an extra space for one line and an overflow for the other line. I think this must be solved in nsInlineFrame. > I don't follow this example. > > Using my proposed scheme, if there are N frames in the line and in the worst > case they're all in the same continuation chain, it takes O(N) time to set up > the hash map, then when we process the first visual frame, we spend O(N) time > scanning the continuation list. Then processing all the other frames takes just > O(1) time each. So overall it's O(N), asymptotically optimal. Here you are assuming that traversing the continuation chain takes O(N), but the the chain in worst case can be much bigger that the line. As I said in the previous example.

(In reply to comment #27) > No! Since the correnct frame can be on another line, then we will have an > extra space for one line and an overflow for the other line. I think this > must be solved in nsInlineFrame. How can the correct frame be on another line? > Here you are assuming that traversing the continuation chain takes O(N), but > the the chain in worst case can be much bigger that the line. As I said in > the previous example. You can stop searching the continuation chain when you reach a frame that isn't in the line (i.e. isn't in the map). So you search the PrevContinuation chain until you find a frame that isn't in the line (or reach the end of the chain), and then you search the NextContinuation chain until you find a frame that isn't in the line (or reach the end of the chain).

> > No! Since the correnct frame can be on another line, then we will have an > > extra space for one line and an overflow for the other line. I think this > > must be solved in nsInlineFrame. > > How can the correct frame be on another line? As you know in RTL, left padding must apply to the leftmost frame of the last line. Now it applies to the leftmost frame of the first line. Take a look at http://lxr.mozilla.org/mozilla/source/layout/generic/nsInlineFrame.cpp#554 It must be changed to something like this: PRBool ltr = (NS_STYLE_DIRECTION_LTR == aReflowState.mStyleVisibility->mDirection); if (nsnull == GetPrevContinuation()) { aMetrics.width += ltr ? aReflowState.mComputedBorderPadding.left : aReflowState.mComputedBorderPadding.right; } if (NS_FRAME_IS_COMPLETE(aStatus) && (!GetNextContinuation() || GetNextInFlow())) { aMetrics.width += ltr ? aReflowState.mComputedBorderPadding.right : aReflowState.mComputedBorderPadding.left; } > You can stop searching the continuation chain when you reach a frame that isn't > in the line (i.e. isn't in the map). So you search the PrevContinuation chain > until you find a frame that isn't in the line (or reach the end of the chain), > and then you search the NextContinuation chain until you find a frame that > isn't in the line (or reach the end of the chain). Yes, You're right. I will implement it.

> > How can the correct frame be on another line? > As you know in RTL, left padding must apply to the leftmost frame of the last > line. Ah, of course. Sorry for being slow.

Haamed, how's it going?

Sorry for delay, I was a little busy. I applied your comments.

No worries, I just want to make sure that your hard work is not lost.

+ /** + * TRUE if this frame is the first visual frame of its continuation chain on + * this line and the chain has some frames on the previous lines. + */ + PRBool mHasContOnPrevLines; + + /** + * TRUE if this frame is the first visual frame of its continuation chain on + * this line and the chain has some frames left for next lines. + */ + PRBool mHasContOnNextLines; Make these PRPackedBool +typedef nsDataHashtable<nsISupportsHashKey, nsFrameContinuationState> nsContinuationStates; I think it would be better to use an nsTHashtable. Make nsFrameContinuationState derive from nsVoidPtrHashKey (because we don't refcount frames so nsISupportsHashkey is not needed). Then Get will give you a pointer to the data in the hashtable so you don't have to Put changed records back. In nsInlineFrame, you can add your new state bits via AddStateBits/RemoteStateBits/GetStateBits so you don't have to add a new field. Define your state bits like #define NS_INLINE_FRAME_HARD_TEXT_OFFSETS above. You can use any bits from 0xFFF00000 that aren't already taken; 0x00400000, 0x00800000, and 0x01000000 would be suitable. + ((nsInlineFrame*)aFrame)->SetBidiLeftMost(isLeftMost); + ((nsInlineFrame*)aFrame)->SetBidiRightMost(isRightMost); Use NS_STATIC_CAST here. + if (aFrame->GetType() == nsLayoutAtoms::inlineFrame) { This isn't right because some classes derived from nsInlineFrame have different types (nsFirstLineFrame, nsPositionedInlineFrame, nsMathMLInlineFrame). Probably the best way to fix this right now is to call aFrame->QueryInterface(NS_INLINE_FRAME_CID, (void**)&testFrame) and see if testFrame is non-null. Other than those minor things, it looks really good!

(In reply to comment #33) > No worries, I just want to make sure that your hard work is not lost. No, I won't give up so soon! I've planned a long term cooperation with mozilla about BiDi stuff! :)

(In reply to comment #34) > Use NS_STATIC_CAST here. > + if (aFrame->GetType() == nsLayoutAtoms::inlineFrame) { > This isn't right because some classes derived from nsInlineFrame have different > types (nsFirstLineFrame, nsPositionedInlineFrame, nsMathMLInlineFrame). So what about it? http://lxr.mozilla.org/mozilla/source/layout/base/nsBidiPresUtils.cpp#475 PRBool IsBidiLeaf(nsIFrame* aFrame) { nsIAtom* frameType = aFrame->GetType(); nsIFrame* kid = aFrame->GetFirstChild(nsnull); return !kid || aFrame->GetStyleDisplay()->IsBlockLevel() || !(nsLayoutAtoms::inlineFrame == frameType || nsLayoutAtoms::positionedInlineFrame == frameType || nsLayoutAtoms::letterFrame == frameType || nsLayoutAtoms::blockFrame == frameType); }

That's too fragile. The QueryInterface approach won't break if a new nsInlineFrame subclass is added.

(In reply to comment #37) > That's too fragile. The QueryInterface approach won't break if a new > nsInlineFrame subclass is added. > So it should be fixed in my patch or in a separate patch?

+ ((nsInlineFrame*)aFrame)->AddStateBits(NS_INLINE_FRAME_BIDI_VISUAL_STATE_IS_SET); You don't need these casts. Add/RemoveStateBits is defined on nsIFrame. + nsFrameContinuationState* frameState; + nsFrameContinuationState* firstFrameState; + + frameState = aContinuationStates->GetEntry(aFrame); Tiny nit: define "nsFrameContinuationState* frameState =" on one line. +// nsFrameContinuationState(nsIFrame* aFrame, PRUint32 aCount) : +// mFirstVisualFrame(aFrame), mFrameCount(aCount) {} Why is this here? + nsFrameContinuationState(const void *aFrame) : nsVoidPtrHashKey(aFrame) {} I think you need to initialize mFirstVisualFrame to null here, otherwise it's used uninitialized. Nearly there! :-)

> + nsFrameContinuationState(const void *aFrame) : nsVoidPtrHashKey(aFrame) {} > > I think you need to initialize mFirstVisualFrame to null here... We initialize mFirstVisualFrame in nsBidiPresUtils::InitContinuationStates() + state->mFirstVisualFrame = nsnull; + state->mFrameCount = 0;

Comment on attachment 225262 [details] [diff] [review] V8 (In reply to comment #41) > We initialize mFirstVisualFrame in nsBidiPresUtils::InitContinuationStates() > > + state->mFirstVisualFrame = nsnull; > + state->mFrameCount = 0; Ah, right. OK. The patch you just attached has a CVS conflict in it. Fix that, and get this checked in! Uri can do it or you can find someone on IRC to help.

I would like to thank Uri and Roc who helped a lot. Uri would you please check in the patch?

Checked in to trunk. Haamed - many thanks for fixing this. Checking in layout/base/nsBidiPresUtils.cpp; /cvsroot/mozilla/layout/base/nsBidiPresUtils.cpp,v <-- nsBidiPresUtils.cpp new revision: 1.80; previous revision: 1.79 done Checking in layout/base/nsBidiPresUtils.h; /cvsroot/mozilla/layout/base/nsBidiPresUtils.h,v <-- nsBidiPresUtils.h new revision: 1.22; previous revision: 1.21 done Checking in layout/generic/nsInlineFrame.cpp; /cvsroot/mozilla/layout/generic/nsInlineFrame.cpp,v <-- nsInlineFrame.cpp new revision: 3.258; previous revision: 3.257 done Checking in layout/generic/nsInlineFrame.h; /cvsroot/mozilla/layout/generic/nsInlineFrame.h,v <-- nsInlineFrame.h new revision: 1.53; previous revision: 1.52 done Checking in layout/generic/nsLineLayout.cpp; /cvsroot/mozilla/layout/generic/nsLineLayout.cpp,v <-- nsLineLayout.cpp new revision: 3.237; previous revision: 3.236 done