The margin-right property is affecting line wrapping of inline elements on all lines, instead of only the last one. For instance, span { margin-right: 10em; } <span> some long text that will span many lines </span> ...will have 10em of right margin on every line, not just after the word "lines". TO REPRODUCE: View attached testcase. EXPECTED RESULTS: The top section should lay out identically to the bottom section. ACTUAL RESULTS: margin-right is affecting every line of the SPAN. TESTED ON: Windows 2000 commercial build 6.0.17.2000072811.

Nominating for nsbeta3. This is a bad CSS1 correctness issue. It is also a regression and is a bug tested on a high profile test suite: the WSP's top 10 for MacIE.

Am going to start with Marc's team for this one.

CC'ing waterson to see if his recent line work caused this regression? Chris, any chance?

Most likely; I'll take a look...

See also related bug 3490, which shows very odd results with negative right margins on inline elements. Chris, if you are not too doomed and can take that bug (bug 3490) I'm sure buster would be very thankful. ;-)

Going to mark as [nsbeta3-], but I'll probably be looking at this while staggering through 3940. There seem to be several problems with margins and borders on inline elements.

Marking rtm-.

any progress with this bug?

This test case illustrates that the problem exists with margin-left, alone.

This fixes margin-left, but not margin-right (that's going to be trickier, because we may or may not wrap the line depending on whether or not the right margin is applied). The fix is to zero the left margin _before_ adjusting the available width. Also, I'm really stumped as to why nsLineLayout::ApplyLeftMargin was trying to determine if the frame was floated. Inline frames that are floated get area frames created for them (see nsCSSFrameConstructor::ConstructFrameByDisplayType), so this code would never be reached. So I removed it. (I suppose I could add an NS_NOTREACHED if people don't trust me!)

Comment on attachment 66576 [details] [diff] [review] fix for margin-left r=dbaron

... although I think it would be a good idea to add an assertion about floating, with a comment that there used to be code to handle it. (Remember when I tried to remove the code that handled incremental reflow during printing? :-)

This test case checks the starting margin for an inline frame: margin-left for left-to-right text; margin-right for right-to-left text.

The above patch supersedes attachment 66576 [details] [diff] [review]: it attempts to handle right-to-left text, as well. The only part of the patch that I'm not sure about is the detection of a right-to-left text frame; specifically, I doubt that I'm correctly taking embedding levels into account (since I don't really understand them). nsLineLayout::CanPlaceFrame had the same vestigial floater detection code, which I've changed to an assertion.

Actually, attachment 66878 [details] [diff] [review] should be a complete fix for this bug and bug 3490, modulo a minor glitch that this patch fixes. (And the question of whether or not I'm detecting the directionality correctly.)

You shouldn't need to take embedding levels into account for this, only the paragraph direction, so the direction tests look OK to me. Does the patch have any impact on bug 121633?

I banged around on bug 121633 for a while: it may be necessary to fix this bug for 121633 to work, but it is not sufficient.

Layout regression tests pass with no bbox errors.

Comment on attachment 66882 [details] [diff] [review] fix looks good r=rbs

Comment on attachment 66882 [details] [diff] [review] fix sr=kin@netscape.com Should we change "left" and "right" in the following comment to "start" and "end"? + // Adjust available width to account for the left margin. The + // right margin will be accounted for when we finish flowing the + // frame.

Right. Thanks!

Fix checked in.

VERIFIED FIXED. Go Chris! :-D