220266 - element height is 0 with overflow hidden, height 0 and padding x px

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P2)

Description

[Ron Derksen]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20030924 Firebird/0.7+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20030924 Firebird/0.7+

If you look at the example URL, you'll see a CSS image replacement technique.
The first example was created by Todd Fahrner and works as it should. The second
example however, doesn't display the image. It did in previous versions of Firebird.

The correct behavior is that the padding is shown, but the height is not. So in
effect, the background is shown in the padding, but the actual text contained in
the element isn't shown.

Reproducible: Always

Steps to Reproduce:
View example url

Comment 1

[alanjstr]

Please verify with the latest Mozilla, too.

Comment 2

[Ron Derksen]

Same result (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a)
Gecko/20030925)

Comment 3

[Boris Zbarsky [:bzbarsky]]

If I use "overflow: -moz-hidden-unscrollable", the page works again.  So looks
like the new way that "hidden" works is wrong here...

Comment 4

[Jesse Ruderman]

Is this the same as bug 219693?

Comment 5

[Mats Palmgren (inactive)]

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

Comment 6

[rob cherny]

This is pretty serious for a lot of designers/developers out there, it's
breaking a lot of sites which are poster childeren for Standards-Based and
Accessible design, such as www.fastcompany.com.  FB/Mozilla used to do this
technique better than IE, now IE looks grand by comparison, and Moz looks ...
just bad.

Comment 7

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

Attachment: testcase with 1em height (baseline)

Comment 8

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

Attachment: testcase with 1em height (baseline)

Something's clearly treating 0 differently from other numbers.

Comment 9

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

Attachment: testcase with 0em height (shows problem)

Something's clearly treating 0 differently from other numbers.

Comment 10

[Pascal Chevrel:pascalc]

affecting this site too :
http://www.fastcompany.com/homepage/index.html

I don't know if it affects 1.5 but in that case shouldn't it be a blocker since
more and more sites are now using this technique for headings and minitabs menus
? In the above example we completely loose the navigation system.

Comment 11

[Boris Zbarsky [:bzbarsky]]

> I don't know if it affects 1.5

It doesn't.  See the dependency on this bug and the checkin dates.  There would
be a slightly more urgent effort to fix this if it affected something other than
a few pre-alpha nightly builds...

Comment 12

[Jemaleddin S. Cole]

Unfortunately, many of the people that are downloading nightly builds are
high-profile designers that blog and have been extolling the virtues of Mozilla
and Firebird for a while now.  While it may seem like a minor bug, it's standing
in the way of advancing highly accessible designs like the one now implemented
at fastcompany.com.

Comment 13

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

I guess we should make the nightly builds crash every 10 page loads so people
don't use them, then.

Comment 14

[Scott Johnson]

Not a good idea.  We need people to run nightlies so that bugs are found and
resolved.  Sure, not everyone should run nightlies, but then the people who
shouldn't be running them usually don't.

Comment 15

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

The point was that people shouldn't take nightlies that seriously, and making
useless advocacy comments, like comment 12, in bugs only makes those bugs harder
to fix, since it's harder to find the technical information within the bug report.

Comment 16

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

The problematic code is the following in nsBoxFrame::Reflow:

  // this happens sometimes. So lets handle it gracefully.
  if (aReflowState.mComputedHeight == 0) {
    nsSize minSize(0,0);
    GetMinSize(state, minSize);
    computedSize.height = minSize.height - m.top - m.bottom;
  }

Comment 17

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

Attachment: patch

Comment 18

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

Comment on attachment 132544 [details] [diff] [review]
patch

I'm not a big fan of this type of code, but at least it shouldn't subtract the
margins off before they've been added.

Comment 19

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

Actually, though, maybe GetMinSize is supposed to include the margins.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 132544 [details] [diff] [review]
patch

r+sr=bzbarsky

Comment 21

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

So, from nsSprocketLayout, it looks like GetMinSize is supposed to include
border and padding (and inset).  However, nsBoxToBlockAdaptor::GetMinSize is a
mess.  nsIBox::AddCSS*Size are documented and assumed to add border and padding,
but they don't.  (Maybe XUL's height and width attributes are supposed to have
it included.)  Beyond that, the |completelyRedefined| business makes little
sense to me.

Comment 22

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

Attachment: patch

I think I'm more comfortable doing this, since I think discontinuities are
obviously wrong.  I have no idea what the rest of the code is supposed to be
doing (and actually changing stuff in nsBoxToBlockAdaptor that I thought would
fix the problem didn't fix it).

Comment 23

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 132548 [details] [diff] [review]
patch

hmm.. yeah, good point about discontinuities...

We should probably document nsIBox at some point and check that callers are
consistent with the documentation and with each other...

Comment 24

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

Fix checked in to trunk, 2003-10-02 15:25 -0700.

Comment 25

[asmith]

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