[INLINE-H] line-breaking bug caused by +ve padding-right on inline elements [LINE] {ll}

NEW
Unassigned

Status

()

Core
Layout: Block and Inline
P3
normal
16 years ago
2 years ago

People

(Reporter: Hixie (not reading bugmail), Unassigned)

Tracking

(Blocks: 2 bugs, {css1, testcase})

Trunk
Future
css1, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Hixie-P2][CSS1-5.5.7] (high profile: css1 test suite), URL)

Attachments

(5 attachments)

(Reporter)

Description

16 years ago
STEPS TO REPRODUCE
   See section 2 paragraphs 2 and 3 of:
      http://www.webstandards.org/css/macie/inlinedemo1.html

   Basically, end padding on an inline element is being treated as an end margin 
   on every line.

This hopefully will be easily fixed using ideas from bug 46918.
(Reporter)

Updated

16 years ago
Keywords: css1, mozilla1.0, testcase
QA Contact: petersen → ian
Whiteboard: [Hixie-P2] (high profile: css1 test suite)

Comment 1

16 years ago
Urp. Did I regress this with my checkin for bug 46918?
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla0.9.9
Whiteboard: [Hixie-P2] (high profile: css1 test suite) → [Hixie-P2][CSS1-5.5.7] (high profile: css1 test suite)
Reassigning to Alex
Assignee: waterson → alexsavulov
Status: ASSIGNED → NEW
Target Milestone: mozilla0.9.9 → Future

Comment 3

15 years ago
Reconfirmed using FizzillaCFM/2002070913. Setting All/All and updating URL.
->Layout: Block&Inline
Assignee: alexsavulov → block-and-inline
Component: Layout → Layout: Block & Inline

Comment 5

11 years ago
This bug still exists in FF2.0
To be precise, padding-right on an inline element adds both the inline padding on the last line, and the same amount of margin-right.

See http://www.altereco.nl/paddingbug-ff2.html
Duplicate of this bug: 386604
So what's the expected rendering when you have, say, 4em of text that can break at the 2em mark, 3em of right padding, and a total width of 5 em?  I assume in that situation we'd want to break the text at the 2em mark, right?

Also note that bug 386604 has some testcases (using padding and nbsp) that could be used as a reftest for this...
Also, setting a border-right-width has the same problem.
(In reply to comment #7)
> So what's the expected rendering when you have, say, 4em of text that can break
> at the 2em mark, 3em of right padding, and a total width of 5 em?  I assume in
> that situation we'd want to break the text at the 2em mark, right?

Yes.
This could be fixed thanks to the backup-and-retry line reflow logic we have now. We'd just stop subtracting inline padding and border-width from the available width.
Assignee: layout.block-and-inline → nobody
QA Contact: ian → layout.block-and-inline
Blocks: 605520

Comment 11

5 years ago
Who do I have to bribe to get this fixed? Or even _confirmed_! It's _still_ a problem. Gecko _still_ adds weird right padding (though it doesn't extend the background into the fake padding, so it's more like a kind of padding/margin hybrid) to inline elements when they break over a line, and it's still not correct.

It's a bug for CSS 1: http://www.w3.org/TR/CSS1/#inline-elements
It's a bug for CSS 2: http://www.w3.org/TR/CSS2/visuren.html#inline-formatting

http://dl.dropbox.com/u/4619191/firefox-getting-it-wrong.png

Padding is being applied at each and every linebreak.

Comment 12

5 years ago
Reduced test based on the original www.webstandards.org/css/macie/inlinedemo1.html

http://www.gtalbot.org/BrowserBugsSection/css21testsuite/Bug-122795-padding-right-on-inline-elements.html

Comment 13

5 years ago
Related test in CSS 2.1 test suite:
http://test.csswg.org/suites/css2.1/latest/html4/c5507-ipadn-r-003.htm
Duplicate of this bug: 1019694
Created attachment 8434908 [details]
testcase for experimenting with padding on inline element

Here's a testcase I've been using to play with this a bit; it allows you to dynamically vary the block width and the padding on a simple inline <span>, to see how line-breaking behaves. The example with padding-left works as expected; padding-right shows clear breakage.
Created attachment 8434909 [details]
bad line-breaking caused by padding-right on inline element

Screenshot showing an example of bad line-breaking as demonstrated with the testcase from attachment 8434908 [details].
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #10)
> This could be fixed thanks to the backup-and-retry line reflow logic we have
> now. We'd just stop subtracting inline padding and border-width from the
> available width.

I've been poking at this a bit. Simply removing the addition of the end padding at
  http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsInlineFrame.cpp#512
does improve things substantially, but it's not quite sufficient.

We get good line-breaks within the bulk of the <span> that has the padding, but if it ends near the right margin (such that there isn't room for its padding to fit), it fails to take the last potential breakpoint within the span, and instead the padding will project into the margin. I think that's probably wrong: the padding at the end of the span is, in effect, an addition to the width of the last character, and so if it doesn't fit, that character also doesn't fit, and we need to take an earlier breakpoint. (See following screenshot.)
Created attachment 8434911 [details]
projecting padding when using the "simple" fix, as per comment 17
Created attachment 8434915 [details] [diff] [review]
possible fix for line-breaking with padding-right on inline element

Here's an attempt to go a bit further, and fix the "projecting padding" described above by backtracking if we find that we're trying to place a frame when we've already overflowed the line width. This behaves nicely with the testcase above, and passes existing unit tests on try: https://tbpl.mozilla.org/?tree=Try&rev=2c6ccdf06cad.

I'm pretty sure there will still be edge cases that are less than perfect, but this seems a substantial improvement on our current behavior.
Attachment #8434915 - Flags: review?(roc)
Created attachment 8434916 [details]
screenshot showing fixed line-breaking

Here's the result of the above patch, using the same test as shown in attachment  8434911 [details] with the partial fix; this time, we get all the correct line-breaks.
Comment on attachment 8434915 [details] [diff] [review]
possible fix for line-breaking with padding-right on inline element

Review of attachment 8434915 [details] [diff] [review]:
-----------------------------------------------------------------

I'm a little confused by this patch. In your testcase, what is the zero-width frame for which CanPlaceFrame is incorrectly returning true?

::: layout/generic/nsLineLayout.h
@@ +537,5 @@
>                         bool aCanRollBackBeforeFrame,
>                         nsHTMLReflowMetrics& aMetrics,
>                         nsReflowStatus& aStatus,
> +                       bool* aOptionalBreakAfterFits,
> +                       bool aAlreadyOverflowed);

Document aAlreadyOverflowed? Maybe the other parameters too if you feel up to it :-)
Attachment #8434915 - Flags: review?(roc) → review-
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #21)

> I'm a little confused by this patch. In your testcase, what is the
> zero-width frame for which CanPlaceFrame is incorrectly returning true?

The text following the <span> with the padding starts with a space. There's a possible line-break after this initial space (before "world"), which gives us a frame for the single space character - which is then trimmed, so the frame ends up zero-width. But at this point the padding from the <span> has pushed us out beyond the allowed width.

That's how the case shown in attachment 8434911 [details] arises, if we only make the change in nsInlineFrame.cpp.
Blocks: 1119636
You need to log in before you can comment on or make changes to this bug.