Closed Bug 54119 Opened 20 years ago Closed 20 years ago

percentage (%) image (img) heights ignore height attribute


(Core :: Layout, defect, P1, major)






(Reporter: bugzillaabuse, Assigned: buster)




(Keywords: testcase, Whiteboard: [rtm++] r=karnaze and attinasi, a=waterson)


(2 files)

BuildID:    2000-09-19-09 (Mac); 2000-09-13-08 (Linux); 2000-09-13-09 (Win98)

IMG HEIGHT not functional.

Reproducible: Always
Steps to Reproduce:
1. Go to aforementioned URL
2. See error.

Actual Results:  Same image appearing regardless of "HEIGHT =" setting

Expected Results:  Should behave properly, as WIDTH does now.  Writing new bug as 
Related to bug #39901 (WIDTH).

Reason for Priority: Will muck up all pages that use this-and that should be 
quite a lot.  Also part of HTML 3.2 spec at least.
Severity: critical → major
Keywords: nsbeta3
cc'ing ekrock for input.  Is this severe enough to consider for beta3, or could
we live with it until rtm? Doesn't seem like a beta-stopper to me.
Dan, is this a recent regression? If so, please mark regression keyword and 
increase priority to P2 at least (high profile backward compatibility) or more 
likely P1 as this will likely affect top 100 sites. 

If this is a recent regression (and it may well be, since image height in HTML 
is fairly basic functionality), then we need to carefully evaluate whether this 
will screw up layout on top 100 sites before letting this slide for nsbeta3, as 
we could embarrass ourselves in that case due to the ubiquity of images on the 
web. It would be worth running that search spider Marcell was using (gerardok or 
paw know about this I think) to see whether image heights with percentages are 
used on top 100 sites.

If on the other hand this is something we just tested for the first time and 
it's not working (seems unlikely), we could let this slip for nsbeta3 and go 
through to rtm. Nominating rtm to make sure this potentially-serious bug doesn't 
fall off radar without more info.
Keywords: rtm
Buster, this should probably rtm+, right?
Assignee: clayton → buster
Priority: P3 → P2
I suggest we get this fixed for rtm, but not hold beta3 for it.
Priority: P2 → P1
Uh, it fails in PR2 actually.  Thing is, this was known, but as far as I knew, it 
was logged under bug 39901.  It turns out that 39901 was *specific to width* and 
no one let me know:).  Anyway, I took the suggestion and filed a new one.  So no, 
not a new issue, but I figure code from the fix that went into 39901 could be 
transposed into this fix, no?
unfortunately, it's not a matter of "what you do for %-width, just do that for 
%-height."  HTML is width-biased, and the code reflects that, so it's slightly 
asymmetric.  In any event, this will be a 1-2 day fix, not much code, but tons 
of testing will need to be done.

I suggest rtm++.
Strongly endorse for RTM+. We're screwing up HTML 3.2 backward compatibility 
here. Last time I checked, there was a *lot* of HTML 3.2 out there on the web 
... ;-> Let's try to get this fixed and checked in on the trunk ASAP and get 
mozilla community volunteers to help pound on this to make sure we're not 
regressing something else when we fix it.
Blocks: html4.01
Keywords: testcase
Adding rtm+.
Whiteboard: [rtm+]
PDT marking [rtm-] because we don't see that severity is bad enough to call P1.
If it really is breaking a ton of pages, it's [rtm need info] at best since no
patch or reviews are available.
Whiteboard: [rtm+] → [rtm-]
I have a partial fix for this that makes images behave properly everywhere
except in tables.  Making them work in tables would take some time.
Even if we can't fix this in tables, fixing it in the simple case would be a 
huge step forward. Steve, could you please get review and super-review and set 
[rtm+] for re-evaluation by PDT for RTM?
clearing rtm- and reverting to [rtm need info] for reconsideration, per ekrock.
 eric says it's better to get a partial fix in than no fix at all, and the
partial fix is very safe.  patch to be attached momentarily.
Whiteboard: [rtm-] → [pdt need info]
patch about to be attached:
karnaze or marc, please review.
waterson, please super-duper-double-dog-dare-ya review.

Note: this is a *partial* fix. It enables %-height on images that are _not_ in
tables.  %-height images in tables still will not get their height set
correctly, probably because they are not getting a final reflow after the cell
decides it's own height.  That problem will have to wait post-rtm.
Attached patch proposed patchSplinter Review
The attached patch looks good to me, although this construct:
is not one I've seen before so I rely on you knowing it is the correct way to
check for a replaced inline frame (it certainly looks like that is what it is

You don't need the last to "else" clauses in the second part of the patch.
You've already set heightUnit to eStyleUnit_Auto. So I'd change it to:

  if (eCompatibility_NavQuirks == mode) {
    aContainingBlockHeight = CalcQuirkContainingBlockHeight(*cbrs);
  else if (NS_AUTOHEIGHT != cbrs->mComputedHeight)
    AContainingBlockHeight = cbrs->mComputedHegiht;

Sound right?

FWIW, there are plenty of other problems with images in tables. :-)
If the above change is indeed correct (buster, karnaze, sanity check me there),
then make it, and a=waterson
marking rtm+ per karnaze's verbal instructions, noting reviews and
super-reviewer in whiteboard.  recommend rtm++.  low risk, high value.  wish I
had time to make them work in tables, but that's a bigger task.
Whiteboard: [pdt need info] → [rtm+] r=karnaze and attinasi, a=waterson
PDT marking [rtm++] but we meant it before when we said this wasn't high
severity, so check in soon since the ++ won't last a long time.
Whiteboard: [rtm+] r=karnaze and attinasi, a=waterson → [rtm++] r=karnaze and attinasi, a=waterson
fix checked into branch
fix now checked into trunk as well.
Closed: 20 years ago
Resolution: --- → FIXED
What is the bug number for percentage heights within tables?
Blocks: 55874
submitted bug 55874 to track replaced elements with %-height not working in
tables.  QA: when verifying, please verify IMG and other inline replaced
elements in BODY, DIV, P, etc.  But do not verify these elements in TABLE.  This
problem is covered in bug 55874 and won't be fixed for NS6 RTM.
Summary: percentage (%) image (img) heights do nothing → percentage (%) image (img) heights ignore height attribute
Sure enoughlooks beautiful:).  Marking VERIFIED.
Keywords: vtrunk
It has come to my attention I'm not *really* supposed to verify the bug, but 
leave the vtrunk KW in there and leave as resolved.  I'll fix this now.
Resolution: FIXED → ---
there was no reason to reopen this bug.  bug 55874 covers all portions of the
fix that are not in the code now.
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
The problem was that I wasn't supposed to verify it without also verifying the 
trunk.  I meant to resolve it again, sorry-I crashed and lost my train of 
thought. -d
*** Bug 56577 has been marked as a duplicate of this bug. ***
Verified Fixed on trunk builds testcase URL displays sized pics properly
linux 101808 RedHat 6.2
win32 101804 NT 4
mac 101804 Mac OS9
Setting bug to Verified and removing vtrunk keyword
Keywords: vtrunk
SPAM. HTML Element component deprecated, changing component to Layout. See bug
88132 for details.
Component: HTML Element → Layout
You need to log in before you can comment on or make changes to this bug.