263374 - Horizontal lines on page (using text-indent and display: block)

Status: VERIFIED FIXED

(Core :: Layout: Block and Inline, defect, P3)

Description

[:Gavin Sharp [email: [e-mail]]]

While viewing the URL given above, I see three horizontal blue lines across the
screen. Seems to be caused by a combination of text-indent: -9999px and display:
block on the li element. I believe the lines correspond to links that have
text-indent set to -9999px.

Will attach reduced testcase and screenshot.

Reproducible on:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a5) Gecko/20041007
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.4) Gecko/20041007
Mozilla/5.0 (Windows; U; Windows NT 5.1; rv:1.7.3) Gecko/20040930 Firefox/0.10.1
(MOOX M3)

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Attachment: Testcase

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Attachment: Screenshot of site showing horizontal lines

Comment 3

[:Gavin Sharp [email: [e-mail]]]

Note also that when selecting the link (via tab), the dotted border indicating
focus spans the width of the page.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Gavin, is the fact that <li> is involved needed to produce the bug?

If you set a smaller text-indent (say -200px), does this still happen?

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Attachment: Testcase #2

No, it does not appear to be dependent on <LI>, and increasing the indent to
-100 has no effect either. See testcase 2.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Attachment: Testcase #3 (reduced)

Another testcase. Seems to be cause by having a link (<A>) inside a container
that has text-indent set to a large enough negative number to make it appear
off screen. It has something to do with the underlining of the links, as you
can see from this new testcase. Notice also that the border that indicates
focus on the link when you click on it spans the width of the browser window.
Might be related.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase that's actually reduced

This testcase demonstrates several aspects of this bug, as well as of
text-decoration rendering in general.

The problem is we decorate the nsLineBox, which can have a negative width.  If
we try to fill a negative-width rect, gfx gets all confused on us.

Per spec, we should be decorating the anonymous inline box that contains the
kids.  This box starts after the text-indent and contains the child margins. 
In other words, in this testcase we should have the following rendering:

First div:  Just the letter "i" underlined
Second div: Just the letter "i" underlined
Third div:  Nothing underlined (since the anonymous box has 0 width)
Fourth div: Nothing underlined (same reason).

I guess what we really need here is the computed text-indent...  If we offset
for it and adjust the width to be painted by it, then check for
non-negativeness we should get the right results.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Same as diff -w

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 162327 [details] [diff] [review]
Same as diff -w

I took the opportunity to factor out the places that were looking for the
containing block into one place (esp. since they were doing it wrong for abs
pos kids of rel pos inlines).

Comment 11

[Boris Zbarsky [:bzbarsky]]

Note that the GetContainingBlockFor() prototype should be returning a non-const
nsIFrame*.  It's fixed locally and I'll make sure to check it in right.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase that tests floats too

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 162327 [details] [diff] [review]
Same as diff -w

r+sr=dbaron, although I'm wondering whether you're sure that all of these
callers can deal with the result being an inline.  (They'll probably get the
wrong width for inlines with continuations -- and it's probably only fixing
that that would cause weird problems.  But it's probably worth filing some
followup bugs.)

Comment 14

[Boris Zbarsky [:bzbarsky]]

The existing callers can deal with it being an inline insofar as we just use the
first-in-flow of a rel pos inline as the abs pos containing block.  So they'll
be looking at the frame we actually use...

If and when we change that, some of these callers will indeed need to be revisited.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Checked in.

Comment 16

[rbs]

Might have been best to drop the helper function in layoututils. It looks a bit
weird in a (transient) reflow'state.

Comment 17

[:Gavin Sharp [email: [e-mail]]]

Verified fixed.
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a6) Gecko/20041125
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a6) Gecko/20041125

Comment 18

[Boris Zbarsky [:bzbarsky]]

I think we want to move toward encapsulating a lot of the logic for these sort
of computations in the reflow state class or some class that reflow state uses
heavily...  So it made more sese to put it there than the general utils class.

Comment 19

[Uri Bernstein]

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

Comment 20

[Boris Zbarsky [:bzbarsky]]

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

Comment 21

[Andrew Schultz]

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