Last Comment Bug 122795 - [INLINE-H] line-breaking bug caused by +ve padding-right on inline elements [LINE] {ll}
: [INLINE-H] line-breaking bug caused by +ve padding-right on inline elements [...
Status: NEW
[Hixie-P2][CSS1-5.5.7] (high profile:...
: css1, testcase
Product: Core
Classification: Components
Component: Layout: Block and Inline (show other bugs)
: Trunk
: All All
: P3 normal with 3 votes (vote)
: Future
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
http://archive.webstandards.org/css/m...
: 386604 1019694 (view as bug list)
Depends on:
Blocks: css2.1-tests 1119636
  Show dependency treegraph
 
Reported: 2002-01-31 09:58 PST by Hixie (not reading bugmail)
Modified: 2015-01-08 19:29 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase for experimenting with padding on inline element (1.55 KB, text/html)
2014-06-05 04:42 PDT, Jonathan Kew (:jfkthame)
no flags Details
bad line-breaking caused by padding-right on inline element (53.73 KB, image/png)
2014-06-05 04:43 PDT, Jonathan Kew (:jfkthame)
no flags Details
projecting padding when using the "simple" fix, as per comment 17 (54.09 KB, image/png)
2014-06-05 04:52 PDT, Jonathan Kew (:jfkthame)
no flags Details
possible fix for line-breaking with padding-right on inline element (5.65 KB, patch)
2014-06-05 04:58 PDT, Jonathan Kew (:jfkthame)
roc: review-
Details | Diff | Splinter Review
screenshot showing fixed line-breaking (53.95 KB, image/png)
2014-06-05 05:00 PDT, Jonathan Kew (:jfkthame)
no flags Details

Description Hixie (not reading bugmail) 2002-01-31 09:58:41 PST
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.
Comment 1 Chris Waterson 2002-01-31 12:34:16 PST
Urp. Did I regress this with my checkin for bug 46918?
Comment 2 Kevin McCluskey (gone) 2002-02-27 13:58:33 PST
Reassigning to Alex
Comment 3 Greg K. 2002-07-11 22:04:31 PDT
Reconfirmed using FizzillaCFM/2002070913. Setting All/All and updating URL.
Comment 4 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2003-03-30 14:31:37 PST
->Layout: Block&Inline
Comment 5 Loek 2006-11-24 02:28:46 PST
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
Comment 6 Boris Zbarsky [:bz] (TPAC) 2007-07-03 10:04:39 PDT
*** Bug 386604 has been marked as a duplicate of this bug. ***
Comment 7 Boris Zbarsky [:bz] (TPAC) 2007-07-03 10:35:11 PDT
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...
Comment 8 Boris Zbarsky [:bz] (TPAC) 2007-07-03 10:36:52 PDT
Also, setting a border-right-width has the same problem.
Comment 9 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2007-07-03 11:15:22 PDT
(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.
Comment 10 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-07-03 15:29:20 PDT
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.
Comment 11 Peter Bright 2012-01-23 17:53:28 PST
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 Gérard Talbot 2012-12-27 18:42:02 PST
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 Gérard Talbot 2012-12-28 21:41:44 PST
Related test in CSS 2.1 test suite:
http://test.csswg.org/suites/css2.1/latest/html4/c5507-ipadn-r-003.htm
Comment 14 Jonathan Kew (:jfkthame) 2014-06-03 11:21:02 PDT
*** Bug 1019694 has been marked as a duplicate of this bug. ***
Comment 15 Jonathan Kew (:jfkthame) 2014-06-05 04:42:25 PDT
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.
Comment 16 Jonathan Kew (:jfkthame) 2014-06-05 04:43:56 PDT
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].
Comment 17 Jonathan Kew (:jfkthame) 2014-06-05 04:51:33 PDT
(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.)
Comment 18 Jonathan Kew (:jfkthame) 2014-06-05 04:52:31 PDT
Created attachment 8434911 [details]
projecting padding when using the "simple" fix, as per comment 17
Comment 19 Jonathan Kew (:jfkthame) 2014-06-05 04:58:20 PDT
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.
Comment 20 Jonathan Kew (:jfkthame) 2014-06-05 05:00:24 PDT
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 21 Robert O'Callahan (:roc) (email my personal email if necessary) 2014-06-05 15:39:04 PDT
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 :-)
Comment 22 Jonathan Kew (:jfkthame) 2014-06-05 23:54:15 PDT
(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.

Note You need to log in before you can comment on or make changes to this bug.