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

RESOLVED FIXED in mozilla1.6alpha

Status

()

Core
Layout: View Rendering
P2
normal
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: Ron Derksen, Assigned: dbaron)

Tracking

Trunk
mozilla1.6alpha
x86
Windows XP
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

14 years ago
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

14 years ago
Please verify with the latest Mozilla, too.
(Reporter)

Comment 2

14 years ago
Same result (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a)
Gecko/20030925)
If I use "overflow: -moz-hidden-unscrollable", the page works again.  So looks
like the new way that "hidden" works is wrong here...
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2

Comment 4

14 years ago
Is this the same as bug 219693?
Depends on: 69355

Comment 5

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

Comment 6

14 years ago
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.
(Assignee)

Comment 7

14 years ago
Created attachment 132469 [details]
testcase with 1em height (baseline)
(Assignee)

Comment 8

14 years ago
Created attachment 132470 [details]
testcase with 1em height (baseline)

Something's clearly treating 0 differently from other numbers.
(Assignee)

Comment 9

14 years ago
Created attachment 132471 [details]
testcase with 0em height (shows problem)

Something's clearly treating 0 differently from other numbers.
(Assignee)

Updated

14 years ago
Attachment #132470 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #132470 - Attachment description: testcase with 0em height (shows problem) → testcase with 1em height (baseline)
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.
> 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

14 years ago
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.
(Assignee)

Comment 13

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

Comment 14

14 years ago
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.
(Assignee)

Comment 15

14 years ago
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.
Assignee: roc → dbaron
Flags: blocking1.6a?
Target Milestone: --- → mozilla1.6alpha
(Assignee)

Comment 16

14 years ago
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;
  }
(Assignee)

Comment 17

14 years ago
Created attachment 132544 [details] [diff] [review]
patch
(Assignee)

Comment 18

14 years ago
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.
Attachment #132544 - Flags: superreview?(bzbarsky)
Attachment #132544 - Flags: review?(bzbarsky)
(Assignee)

Comment 19

14 years ago
Actually, though, maybe GetMinSize is supposed to include the margins.
Comment on attachment 132544 [details] [diff] [review]
patch

r+sr=bzbarsky
Attachment #132544 - Flags: superreview?(bzbarsky)
Attachment #132544 - Flags: superreview+
Attachment #132544 - Flags: review?(bzbarsky)
Attachment #132544 - Flags: review+
(Assignee)

Comment 21

14 years ago
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.
(Assignee)

Comment 22

14 years ago
Created attachment 132548 [details] [diff] [review]
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).
(Assignee)

Updated

14 years ago
Attachment #132544 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #132548 - Flags: superreview?(bzbarsky)
Attachment #132548 - Flags: review?(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...
Attachment #132548 - Flags: superreview?(bzbarsky)
Attachment #132548 - Flags: superreview+
Attachment #132548 - Flags: review?(bzbarsky)
Attachment #132548 - Flags: review+
(Assignee)

Comment 24

14 years ago
Fix checked in to trunk, 2003-10-02 15:25 -0700.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Flags: blocking1.6a?
Resolution: --- → FIXED

Comment 25

14 years ago
*** Bug 220667 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.