Closed Bug 85016 Opened 23 years ago Closed 23 years ago

[FIX]WRMB:elements (such as images) have zero dimensions when percentages are used on page with no DOCTYPE specified

Categories

(Core :: Layout, defect, P2)

defect

Tracking

()

VERIFIED FIXED
mozilla0.9.5

People

(Reporter: apardo, Assigned: attinasi)

References

()

Details

(Keywords: topembed, Whiteboard: PDT+)

Attachments

(7 files, 1 obsolete file)

The texto of this page has a GIF image, that is not seen in the display of the page.
confirmed win98 build:2001060820
The image is sized to 'width="95%" height="65%"'

Unfortunately, the containing block for this image contains _just_ the image.  
So it's sized to 65% of auto, auto defaults to 0, and the image comes up zero-
height.

setting qa to hixie for verification, but this looks like a mistake in the HTML 
code to me.
QA Contact: doronr → ian
since there's no DOCTYPE, shouldn't this be rendered in Quirks mode and thereby 
render the image (im)properly?
Hmm. True.  And this is not 74313 since that's Unix-only....  
Keywords: qawanted
Mail from reporter:

Yes, your sugestion works: if I delete the size of the image, it is
displayed correctly.
Also works adding
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
at the top of the code, like other mail sugested me.
Status: UNCONFIRMED → NEW
Component: Browser-General → Layout
Ever confirmed: true
OS: Windows NT → All
Hardware: PC → All
Summary: GIF image not seen → image has zero dimensions when percentages are used on page with no DOCTYPE specified
reassign
Assignee: asa → karnaze
QA Contact: ian → petersen
See bug 85516. It looks like the problem is causes by the combination of a
percentage scaled image nested in another block element. The DOCTYPE didn't fix
anything with the images in the testcases on bug 85516 using Mozilla 2001062211
on Win2k.
Blocks: 81477
Another example, with a jpg image scaled with percentages in a <p></p>
environment. Adding DOCTYPE does not seem to help.

http://users.skynet.be/www/todo/etentje.htm

(build 0.9.2 on GNU/Linux)
Yes, I would say this problem is related to 85516
Severity: normal → major
Chris: take a look at all of the dependents of bug 81477.
The linked to page appears to be rendering correctly in the latest build
(2001071214/WinNT).
The example given by TGKe, however, is stilling showing nothing
Taking - see bugscape bug 8794 for testcase (topembed)
Assignee: karnaze → attinasi
Keywords: qawantedtopembed
Attached file reduced testcase (for debugging) (obsolete) —
Bug 85516 is a dup of this, and has some good testcases
Status: NEW → ASSIGNED
*** Bug 85516 has been marked as a duplicate of this bug. ***
Why is the bug with "some good testcases" marked as a duplicate of one with
almost none? Additionally, note that bug 85516 shows that the DOCTYPE has no
effect on the bug. I ran one of the testcases on bug 85516 through the HTML
validator at W3C.org, and it came out clean, but it still doesn't display correctly.
> Why is the bug with "some good testcases" marked as a duplicate of one with
> almost none?


They are dups. It does not matter which one is dup'd to which - they are linked
by the duplication process, and after a fix all of the dups have to be checked
anyway. In this case I just dup'd the later one to the earlier one, but it
doesn't really matter (to me at least - let it be know if you feel differently).
Marc -- I am bumping up priority on this one (see bugscape 8794)
Priority: -- → P2
Summary: image has zero dimensions when percentages are used on page with no DOCTYPE specified → WRMB:image has zero dimensions when percentages are used on page with no DOCTYPE specified
OK, I'm also assigning a milestone, just to be clear on the target.
Target Milestone: --- → mozilla0.9.4
So my current thinking is that a container with ONLY percentage-height children
has to get sized, in quirks mode, to 100% of it's container (and if that is the
body, 100% of the viewport). This seems to be IE's behavior anyway.

Any thoughts? I'll attach a testcase in a bit...
Further refinement, based on some more testing in IE: It looks like an IMG with
a percentage height in a block with unconstrained height gets its computed
height basis from the closest containing block that has a height (rather than
the immediate containing block, which has no height). For example:

<div style="border: 1px solid red;">
 <img height="100%" 
      src="http://home.netscape.com/ex/shak/home2001c/images/nslogo_t.gif">
 <img height="100%" 
      src="http://home.netscape.com/ex/shak/home2001c/images/nslogo_t.gif">
</div>
Each image ends up the height of the *viewport*, so the DIV that contains them
gets the height of the two images together, or double the viewport height.
Tables, however, stop the search for a sized containing block.
Moving to mozilla0.9.5
Target Milestone: mozilla0.9.4 → mozilla0.9.5
I attached a patch that tweaks the existing CalcQuirkContainingBlockHeight
method to allows for percentage based elements in deeply nested blocks, instead
of just elements that are directly in teh body or html element. I discussed the
original patch with karnaze and he thought that he did it that way just to
handle a special table case. I tested the patch with images and tables, nested
in divs and other elements, and our compatibility with IE is better now, and the
URL and testcases are fixed.
Summary: WRMB:image has zero dimensions when percentages are used on page with no DOCTYPE specified → [FIX]WRMB:image has zero dimensions when percentages are used on page with no DOCTYPE specified
Blocks: 96795
Comment on attachment 48247 [details] [diff] [review]
Patch: Reworked CalcQuirkContainingBlockHeight to allow nesting of unconstrained blocks for IE compatibility

r=karnaze
Attachment #48247 - Flags: review+
Comment on attachment 48247 [details] [diff] [review]
Patch: Reworked CalcQuirkContainingBlockHeight to allow nesting of unconstrained blocks for IE compatibility

forgot to add that r= mandates running table regression tests.
Comment on attachment 48247 [details] [diff] [review]
Patch: Reworked CalcQuirkContainingBlockHeight to allow nesting of unconstrained blocks for IE compatibility

sr=waterson. Might want to put |continue| statements on a different line so you can set a breakpoint there.
Attachment #48247 - Flags: superreview+
Good suggestion - will do.
table and block regression tests completed and passed.
*** Bug 42543 has been marked as a duplicate of this bug. ***
checked into trunk
Whiteboard: checked into trunk
someting - possibly this - just made loading http://linuxgames.com/ very slow,
and also faulty. White "border" images around stories now look bad. Scrollin
down in page after it's seemingly loaded, i see the the borders render terribly
slow while i watch.
Checking it out - thanks rkaa
The patch does slow down the page at linuxgames.com a little for me, but I am
not seeing the rendering problems. Also, I put a break in the code that was
altered in the patch, and it is not hit during scrolling around at all. It could
be that you were scrolling while the page was still loading, however, and the
rendering of all of those images for the borders was competing with the layout,
which admitedly does seem slower.

From my instrumentation of the modified routine, I see about 3 frame levels
being checked for the containing block height on that page, with a few 5-level
checks. That routine used to max out at 2 levels, usually 1. This extra
traversal will cost somewhat in performance, but it should not be that much.
There is also an expensive #ifdef DEBUG that I added that could slow it down
too, if you are using a DEBUG build - can you try removing that? It seems to
speed it up a little for me. I'd be happy to turn off the DEBUG checks if that
help enough...
It looks like we are computing the containing block height even when we don't
need it - I think I can hack the patch to avoid the extra traversals if the
actual computed height of the 'virtual' containing block is not needed, like
when the frame being reflowed has an explicit height. 
The layout glitch was a DTD issue as you saw, dbaron filed bug 98977.
I build for speed, non-debug, -O2 etc, and the slowdown is noticable using a
common P3/500.
I have a new patch for this problem. Basically, I restrict the change I made to
a specific case, where we know that the actual computed size of the containing
block is really needed. Otherwise, the method does what it always did, and the
performance hit is totally mitigated. I'll attach a patch against the trunk
hoping that RKAa can test it out.
Patched with attachment 48887 [details] [diff] [review], Linuxgames is fast again and no visible lag when
scrolling. Lines are already in place by the time i get to them. (Roamed other
sites as well, NOT that they were affected: cnn.com, www.vg.no, ximian,
dagbladet.no, wired, pcworld, zdnet, cnet, microsoft: all good and speedy)
Could you make the |aRestrictToFirstLevel| parameter always be explicit instead
of using a default parameter (just so nobody wastes time searching through the
file for an overloaded version). Also, maybe elaborate a bit (in comments) why
one might or might not care to restrict the search to one level or not? I.e.,
why is it that we don't need to search >1 level when computing the height from
ComputeContainingBlockRectangle()?
Comment on attachment 48887 [details] [diff] [review]
New Patch (against trunk): restrict the previous patch to the specific case where the extra work is needed

r=karnaze
Attachment #48887 - Flags: review+
Chris, I added the comments, and made the argument required. To be honest, I'd
like to always do the containing block height computation the same way, and walk
up the tree until a real height is found, but this way it restricts the extra
walking to the one case where we know we need it - %-height images in 
unconstrained blocks. 

Do you (or anyone else) know if we can skip the computation of the containing
block height altogether for cases where the contained element does not need it?
It seems like a waste to init a value that is not needed. I'd like to play with
that when I have some more time.
*** Bug 96505 has been marked as a duplicate of this bug. ***
Comment on attachment 48940 [details] [diff] [review]
Updated patch addressing waterson's comments

sr=waterson
Comment on attachment 48940 [details] [diff] [review]
Updated patch addressing waterson's comments

sr=waterson
Attachment #48940 - Flags: superreview+
I don't know when we could skip containing block height. Maybe we ought to just
file another bug to investigate that someday.
Bug 99219 opened for future investigation into the possibility of eliminating
the computation of the containing block height (or more generally, the size).

Trunk patched. Now ready to apply the newest fix to 0.9.2 branch. 

Also, nominating for 0.9.4 for consideration there.
Keywords: mozilla0.9.4
Checked into 0.9.2 branch, waiting for approval for 0.9.4 branch
Whiteboard: checked into trunk → checked into trunk, 0.9.2 branch
Attachment 44353 [details] on bug 85516 is still breaking on 2001091003-trunk. Should this
behavior be affected by the patch, or not? All other testcases for that bug work
as expected after the patch was applied.
Does anyone know if bug 96795 is a dupe of this bug?
yes, bug 96795 was a dup of this one, already dup'd - thanks.
*** Bug 99494 has been marked as a duplicate of this bug. ***
Nominating for PDT - shuold get into the eMojo thingy I think.
Whiteboard: checked into trunk, 0.9.2 branch → PDT
*** Bug 99795 has been marked as a duplicate of this bug. ***
*** Bug 81477 has been marked as a duplicate of this bug. ***
expanding summary since dependent bug was duped.
Summary: [FIX]WRMB:image has zero dimensions when percentages are used on page with no DOCTYPE specified → [FIX]WRMB:elements (such as images) have zero dimensions when percentages are used on page with no DOCTYPE specified
Marking nsbranch+. Fix is ready to be checked into 0.9.4 branch
Keywords: nsbranch+
PDT+. Let's get it checked in ASAP.
Whiteboard: PDT → PDT+
Chk'd into 0.9.4 branch - marking FIXED
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Image tag when used with Percentage values don't work for height but it works 
fine with width.
(Ref Bug 84359 is resolved as duplicate of Bug 81477 and 
Bug 81477 is resolved as duplicate of Bug # 85016 which says its fixed) but the 
actuall porblem is not solved,
Im reopening this bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
test on build ID 2001-09-19-05-0.9.4  
platfrom :windows 2k
Moied, I just checked with 0.9.4 build and it works for me. What testcase or URL
are you trying? Didi you try the attachement to this bug at
http://bugzilla.mozilla.org/attachment.cgi?id=47687&action=view

Tested with todays CVS builds, Win2K and Linux
was the performance fix for linuxgames.com backed out again? The white borders
are very slow - noticed some days ago.
RKAa - trunk or 0.9.4 branch? I never backed it out, and it is still in the
trunk according to lxr.
current CVS. I deleted all in dist and rebuilt now, then deleted cache: it got
faster again. False alarm then. I seem to remember it rendering quicker but
testing it now it was acceptable, and much faster than  before the rebuild.
Confirming that testcases attached above is working with downloaded build
(20010920) as well as my CVS build. Marking FIXED again. If there is still a
perceived problem, please indicate the URL or testcase that is failing, and the
build it is failing on.
Status: REOPENED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
The testcase is rendered properly using the Sept 25th branch Mac OS X
(2001-09-25-05) and Sept 24th branch Linux (2001-09-24-04) build. The image in
the testcase fails to appear in the Sept 25th branch or trunk Windows builds.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 46975 [details]
reduced testcase (for debugging)

testcase is obsolete - see later testcase that uses publicly available image src (thanks basic)
Attachment #46975 - Attachment is obsolete: true
I just tried the new testcase (id=50796) with a brand-new CVS branch build
(11:00am 09-25) and it works fine. Petersen, please recheck with tomorrow's
build - I am suspecting some unrelated problem (possibly inaccessability of
bugscape?).
Verified with the second testcase provided under Windows ME. Tested on branch
build  (2001-09-27-05).
Keywords: vtrunk
Based on petersen's comments and my own findings, marking FIXED
Status: REOPENED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
Verified on Oct 22nd trunk builds.
Status: RESOLVED → VERIFIED
Removed vtrunk
Keywords: vtrunk
according to the testcase of 47687,

for an image with 100% percent height which   is inside a <div> with no css height,
  in strict mode, img use its original height,
  in quirk mode, img use viewport's height.

for an image with 100% percent height which   is inside a <td> with no css height,
  in strict mode, img use its original height,
  in quirk mode, img's height is set to zero.

if replace img with iframe, how it should like?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: