131065 - [strict] Applet/Object data is not rendered when Height attribute is assigned a percentage

Status: VERIFIED FIXED

(Core :: Layout, defect, P2)

Description

[Chris Petersen]

Build: 2002-03-14-08
Platform : All
Expected Results: Object content should render based on the height (percent)
attribute
What I got: Object content is not rendered 

Steps to reproduce:

1) Download both attached test cases
2) Open object_percent_w_h_text.xml
3) The test case contains two objects. First object doesn't render it's content
but the second object does.

Comment 1

[Chris Petersen]

Attachment: Object test case using percentages for width and height

Save this testcase

Comment 2

[Chris Petersen]

Attachment: XML file that is inserted into first testcase

Save this testcase

Comment 3

[Chris Petersen]

Changing Priority to P3

Comment 4

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Hint: if you first upload the attachments, get their URLs, and upload the actual
test file with the correct URLs we wouldn't need to download the tests first and
edit them by hand to include the correct filenames...

Comment 5

[Chris Petersen]

After further research, this problem is related to standards mode. If I place a
strict DTD in a HTML file, I get the same results.

See the two applet test attached:

One with WIDTH percent and the other with HEIGHT percent.

Comment 6

[Chris Petersen]

Attachment: Width attribute specified at 50%.

HTML Doctype is STRICT. Width is percent and applet is displayed.

Comment 7

[Chris Petersen]

Attachment: Height attribute is specified at 50%

HTML Doctype is STRICT. Height is percent and applet is not displayed even
thought applet is initialized.

Comment 8

[Chris Petersen]

This seems to be a regression since the applet height test is working in a older
trunk build (Feb 20th build)

Comment 9

[Chris Petersen]

Changing summary

Comment 10

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Reassigning to layout. My question is: is this intentional in strict mode?

Comment 11

[Chris Petersen]

Thanks a good question... It did work before but maybe we were doing the wrong
thing ? Need to research .

Comment 12

[Peter Lubczynski]

Taking bug...

Might be a regression or fallout from bug 106602.

Comment 13

[Heikki Toivonen (remove -bugzilla when emailing directly)]

This is not XML specific, modifying Summary to reflect this.

Also really reassigning to Peter.

Comment 14

[Denis Antrushin]

This bug is a dup of bug 108831. Either one of these two can be closed

Comment 15

[Denis Antrushin]

Oops, these may be not quite dups. Bug 108831 is about applet with percent height
attribute not displayed in table...

Comment 16

[Peter Lubczynski]

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

Comment 17

[Peter Lubczynski]

nominating nsbeta1

Comment 18

[Peter Lubczynski]

removing nominations, I need some clarification if this is really a bug

I got a tip that in strict mode, the body's height is zero. Therefore, it looks
like we are doing the right thing in these testcase by making the height of the
applet 0. If you put a height on the BODY, the applet gets displayed the right
height.

Could someone more familar with XHTML and strict mode advise?

Comment 19

[rubydoo123]

Since this works fine in quirks mode, which most pages are rendered in, I am 
pushing out to 1.1. The exposure to this regression is relatively low

Comment 20

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

The body's height isn't really zero, but according to CSS, percentage heights
shouldn't do anything if the parent's height is 'auto':

http://www.w3.org/TR/REC-CSS2/visudet.html#the-height-property

Tables have a bunch of code to make HTML-specified percentage heights act
differently from this, though.

Comment 21

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

Attachment: possible fix

This fixes the testcase.  What will it do to the quirks mode case?  (What was
our behavior in the quirks mode case?)

Comment 22

[Denis Antrushin]

I don't see anything on
http://www.w3.org/TR/REC-CSS2/visudet.html#the-height-property that says that
% height shouldn't do anything if  parents' height is auto. 
It says:
"The percentage is calculated with respect to the height of the generated box's
containing block. If the height of the containing block is not specified
explicitly (i.e., it depends on content height), the value is interpreted like
'auto'. "
And auto height is an intrinsic height, isn't it?


Also, applet's height parameter has nothing common to css
What the reasons of not showing applet in the following html :
<html>
<body>
<table border="1">
<tr>
<td><applet code="pabanner.class" width=50% height=50%></applet></td>
</tr>
<tr>
<td>blah-blah</td>
</tr>
</body>
</html>

Also note the asymmetry between width and height params.
Mozilla correctly compute width, but not height

Comment 23

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

For some things, 'auto' is an intrinsic height.  However, it's not 50% of
anything if it was specified as 50%.

Does the patch not fix that testcase?

Comment 24

[Denis Antrushin]

Applet inside of table? Nope.

It still hits last else and its meaning completely escapes me
+    else if (NS_UNCONSTRAINEDSIZE != aReflowState.mComputedHeight)
+      // Unconstrained percent case.
+      aMetrics.height = aReflowState.mComputedHeight;
+    else if (mFrames.FirstChild())
+      // This will be used only by |HandleChild|.  Otherwise it wouldn't
+      // be an appropriate result from |GetDesiredSize| (which is also a
+      // virtual method on |nsLeafFrame|, but |nsObjectFrame| doesn't
+      // derive from |nsLeafFrame|).
+      aMetrics.height = NS_UNCONSTRAINEDSIZE;
+    else
+      aMetrics.height = 0;
       ^^^^^^^^^^^^^^^^^^^ why zero????
In the old html world 50% height in unconstrained table means 50% of content
(browser window, whatever) area, isn't it?

Comment 25

[Peter Lubczynski]

Plugins don't really have any way currently of telling the browser their
"intrinsic" size. I think that's what nsIPluginInstancePeer::SetWindowSize and
NPPVpluginWindowSize were intended for.

Comment 26

[Denis Antrushin]

Proposed patch also doesn't fix xhtml testcase. I tried <applet height=90%> (w/o
tables) with <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
"DTD/xhtml1-transitional.dtd">

Comment 27

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

Zero because the old code said zero, I can't think of anything better, and we
have to pick something.

We have some hacky code somewhere to do the percent-in-the-old-world emulation.
 If someone wants to figure out how to use it, go ahead.

Comment 28

[Denis Antrushin]

Why not to apply percentage to window (content area or whatever it could be
called) size?

Comment 29

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

That would be incorrect for any height style specified through CSS.  It suggests
we really should be mapping HTML percentages for height to different units.

Comment 30

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Note about XHTML testing: there are only two things that make us process XHTML.
The document must have an XML mime type, and the namespace for the elements must
be the XHTML namespace. In other words, if you serve with HTML mime type we will
treat it as regular HTML, and if you serve with XML mime type but forget XHTML
namespace we don't recognize it as XHTML either. So, a DOCTYPE does not matter
at all here.

Comment 31

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

It matters for quirks vs. strict mode.  See http://mozilla.org/web-developer/quirks/

Comment 32

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

er, http://mozilla.org/docs/web-developer/quirks/

Comment 33

[Patty Mac]

Since Patrick B. said bug: 119352 is more likely a layout issue, so I close the 
bug: http://bugzilla.mozilla.org/show_bug.cgi?id=119352 
and add more info into this bug.

Seen on Mac OS 10.1.3 (2002-04-16-03-TRUNK) with MRJ v1.0b2.

Steps to reproduce:

1. Launch netscape.
2. Click on this url:
http://www.mozilla.org/quality/browser/front-end/testcases/oji/test7.html

Notice the applets height=10%, applet height=5%, applet height=3% are not
loading...

Comment 34

[Denis Antrushin]

Note, that applets mentioned in comment #33 are inside of unconstrained table.
Quirks mode.

Comment 35

[Peter Lubczynski]

Strange results (on WinXP w/debug build) on pmac's testcase:

* JRE 1.3.1_02 hangs Moz
* JRE 1.4 seems to layout it our correctly in Moz
* IE 6 only renders the top applet
* 4.x doesn't seem to be scaling the applets correctly either

Best results (near perfect) with JRE 1.4 and Moz

Comment 36

[Denis Antrushin]

Solaris and Linux with java 1.3.1 : only top applet is shown

Comment 37

[rubydoo123]

Just talked this through with karnaze and peterl. With the applet in a table 
where the cell does not have a defined height. Mozilla renders the content the 
same as IE and Opera. Since we are all rendering the test the same, I am not 
convinced this is a bug.

Comment 38

[rubydoo123]

With the applet outside the table, in strict mode - IE and Opera render the 
applet. We do not, since we are not consistent, we should look into a possible 
solution

Comment 39

[Brian Nesse (gone)]

Chris says this worked on 2/20 builds... broken by 3/14. If we narrow that
window down, maybe we can identify what change caused this... and the reason for
that change...

Comment 40

[Peter Lubczynski]

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

Comment 41

[Peter Lubczynski]

This appears to be causing a crash in Linux in bug 138768:

if you enter the url of the flash-file ( http://www.deadtroll.com/trollboot.swf )
mozilla doesn't crash. So I saved the html-file and the swf-file to my
home-directory and played a bit with them.

In the original html file is the line
<embed src="trollboot.swf" width="95%" height="95%" align="TOP"
type="application/x-shockwave-flash" play="true">

If you remove the height and the width attribute from this line, mozilla won't
crash and will display the flash-animation.

Hope that helps,
Martin



------- Additional Comment #8 From Andrew Schultz 2002-04-21 07:38 -------

stacktrace:
#0  0x41af272b in XtAddTimeOut () from /usr/X11R6/lib/libXt.so.6
#1  0x41af2887 in XtAppAddTimeOut () from /usr/X11R6/lib/libXt.so.6
#2  0x452a72cc in morkUsage_g_static_init_done () from 
               /usr/lib/netscape/plugins/libflashplayer.so

build 20020415 works, 20020417 does not.

Comment 42

[Andrew Schultz]

this actually started happening between 20020305 and 20020307.

testcase from bug 138786: atachment 80280

Comment 43

[Denis Antrushin]

removing myself from cc:

Comment 44

[Andrew Schultz]

comment 12 is correct... this is a regression from bug 106602.
keyword crash and severity critical since Flash causes a crash (bug 138786)

Comment 45

[Andrew Schultz]

for attachment 80280 [details], the patch here does not help because mFrames.FirstChild()
is 0.

Comment 46

[karnaze (gone)]

The applet in the 4th attachment is not showing up because it has a strict dtd 
and CalcQuirkContainingBlockHeight does not get called in nsHTMLReflowState.cpp 
in standard mode. It would be easy to change this, but it would violate the 
standard (but of course we don't display the applet even though IE and Opera 
do, because we support the standard). 

Are there any strong opinions one way or the other?

Comment 47

[Marc Attinasi]

Standard mode documents should be treated as standard mode documents, as far as
is possible. Can't we just evangelize the author to fix their page? Oh, wait,
it's the US government...

I fear that doing the Quirk-mode computation in strict mode for all elements
would seriously jeopardize the efficacy of the standards, for whatever that is
worth. If we just did it for Object elements it would be less bad though.

Peter, you ripped out some code for bug 106602 - what about using that to
compute the height if the reflow state provides a height of '0'?

Comment 48

[rubydoo123]

Marc -- how would they properly code their pages? No harm in asking htem to fix
their code

Comment 49

[Andrew Schultz]

note that this is a crasher with Flash.  That is a bug with Flash, but we should
not punish Mozilla users for this if we don't have to.

Comment 50

[Marc Attinasi]

The page should specify a fixed size for the container of object. They could put
in a DIV with a specific height, for example. Or, they could specify the height
of the object in pixels instead of percent. That would take care of the problem
I think.

Comment 51

[Andrew Schultz]

I have been unable to reproduce the crash with other .swf flash movies...

Comment 52

[Eric.Zurcher]

I'm seeing a problem which is probably related to this bug, although it is
possibly separate. I find with Mozilla 1.0 RC3 on Windows 2000 that an applet
which: (1) is embedded within a <p> or <div>; (2) expresses its height as a "%";
and (3) has either align="left" or align="right" is displayed within a window
with a height of 0, making it invisible. The applet is apparently running
correctly, but its display cannot be seen.

I do not see this problem with the Linux version of Mozilla.

Time is pressing at the moment, but I shall try to produce a test case in the
next day or two. However, I do see the same symptoms with the existing "Height
attribute is specified at 50%" attachment - that is, the applet runs within a
window of height 0.

Comment 53

[Eric.Zurcher]

Attachment: Shows applet height problem in Win32 Mozilla

The applet within this document receives a height of 0 under Mozilla 1.0 on Win
2K.

Comment 54

[Eric.Zurcher]

Attachment: Does not show applet height problem in Win32 Mozilla

Here the applet displays correctly with Mozilla 1.0 on Win 2K. Note that the
only signficant difference from the preceeding attachment is the way in which
the <p> tag before the applet is closed!

Comment 55

[rubydoo123]

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

Comment 56

[Peter Lubczynski]

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

Comment 57

[Andrew Schultz]

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

Comment 58

[rubydoo123]

this should have been marked as nsbeta1- already

Comment 59

[Christopher Hoess (gone)]

Adding timeless to cc, he was triggering this bug with a testcase the other 
night.

Comment 60

[rubydoo123]

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

Comment 61

[Andrew Schultz]

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

Comment 62

[rubydoo123]

adding nsbeta1+

Comment 63

[Mike Kaply [:mkaply]]

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

Comment 64

[Mike Kaply [:mkaply]]

http://www.timeticker.com is another site that has this problem.

Comment 65

[Mike Kaply [:mkaply]]

This broke between March 6 at 4PM and March 7 at 8AM

http://bugzilla.mozilla.org/show_bug.cgi?id=106602

Is definitely the prime candidate as it went in during this time.

Comment 66

[Andrew Schultz]

yes, it is.  see comment 44

Comment 67

[Mike Kaply [:mkaply]]

This is not a standards issue at all I believe.

Just having <EMBED HEIGHT=100%> causes displaying flash to fail.

Why should this be a standards issue?

Comment 68

[Jerry]

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

Comment 69

[Jerry]

This bug has so many testcase, let me try to make it clear.
Testcase 1 and testcase 2 are not suitable here since it is not xhtml specific,
so just skip it.

Testcase 5 and testcase 6 are not suitable here also, they are all in quirk mode.
In Testcase 5, applet is after '<p/>', so when parser make content tree,applet
will be treated as <p>'s childen,just like this
<html><body><p><applet height="50%"..></p>,
if reporter think it is a problem, please file another bug about it, its catalog
should be parser.

Testcase 3 is about width. 

So I think Testcase 4 is suitable here.

Comment 70

[Jerry]

Attachment: Patch 0.1

I make patch 0.1 according to comments #c47.
just call quirk method for objectframe in strict mode, 
it wont hurt other frame.

Comment 71

[Jerry]

As Testcase 5 and Testcase 6 are caused by "<p/>", they are not suitable here.
I have filed another bug 162441 for it.
So I mark testcase 5 and testcase 6 as obsolete.

Comment 72

[Joshua Xia]

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

Comment 73

[Jerry]

Attachment: patch 0.2 for discuss

Comment 74

[Jerry]

Patch 0.2 is in the different way from patch 0.1
According to  comment #44, I made patch 0.1

And since this bug is regression of bug 106602
So I made Patch 0.2 for discuss. 

Comment 75

[Jerry]

Here is some comments for patch 0.2
The case of bug 106602 is quite different from bug 131065.
bug 106602 is related with percent height objectframe in TableCell, which will
cause special height reflow of table.
nsObjectFrame::Reflow() will be called three times.
During the third reflow(special height reflow), reflowstate will set
mComputedHeight as percent*Containheight there,
then call nsObjectFrame::Reflow().
so in function body of nsObjectFrame::GetDesiredSize(), it will use
mComputerHeight as desired height.

bug 131065 is related with object in unconstraned height.
it will call nsOjectFrame::Reflow() only once, to set desired height in
nsObjectFrame::GetDesiredSize().

So patch 0.2 calculate its parentframe, if it is TableCell, follow the logic of
106602 fix.
if not, follow the old logic before fix of bug 106602.

Comment 76

[karnaze (gone)]

This needs to include nsLayoutAtoms::bcTableCellFrame also. nsTableCellFrame.h
defines the macro IS_TABLE_CELL which you may want to copy.

+      if (fType.get() == nsLayoutAtoms::tableCellFrame) {
+      	if (aReflowState.mComputedHeight != NS_UNCONSTRAINEDSIZE)
+          aMetrics.height = aReflowState.mComputedHeight;
+      }
+      else {

The XXX comments are valid - we are propogating incorrect code here. Also, what
should happen when an object is inside something inside the body and there are
other elements inside the body?
+        nsRect rect;
+        aPresContext->GetVisibleArea(rect);
+        nscoord h = rect.height;
+        // now make it nicely fit counting margins
+        nsIFrame *containingBlock = nsnull;
+        if (NS_SUCCEEDED(GetContainingBlock(this, &containingBlock)) &&
containingBlock) {
+          containingBlock->GetRect(rect);
+          h -= 2*rect.y;
+          // XXX: this math seems suspect to me.  Is the parent's margin really
twice the x-offset?
+          //      in CSS, a frame can have independent top and bottom margins
+          // but for now, let's just copy what we had before
+        }
+        aMetrics.height = NSToCoordRound (factor * h);
+      }
+    }
   }

Comment 77

[Jerry]

Attachment: patch 0.21 (add bcTableCellFrame)

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
	"http://www.w3.org/TR/1999/REC-html401-19991224/strict.dtd">
<html>
<head>

<title>HTML Functional -- APPLET - WIDTH=percent</title>
<style type="text/css">
html {background-color: olive}
.pile {
left: 2in;
top: 2in;
width: 3in;
height: 3in;
}
</style>
</head>
<body>
<p>The banner applet will be displayed with a width of
50%.</p>
<div class="pile">
<applet code="parbanner.class"
codebase="http://mozilla.org/quality/browser/standards/html/images/"
width="50%"  height="80%">
<param name="message" value="Scrolling banner text !!!" />
</applet>
</div>

<applet code="parbanner.class"
codebase="http://mozilla.org/quality/browser/standards/html/images/"
width="50%"  height="80%">
<param name="message" value="Scrolling banner text !!!" />
</applet>

</body>
</html>

another testcase for karnaze's comments.
it works fine with patch 0.21

Comment 78

[karnaze (gone)]

Attachment: test case with body having unequal top an bottom margins

Comment 79

[karnaze (gone)]

Attachment: revised patch to handle unequal top and bottom margins in containing blocks

Comment 80

[karnaze (gone)]

The revised patch passes the regression tests. I'm taking the bug to try to get
it checked in before the midnight deadline. 

Comment 81

[karnaze (gone)]

Attachment: test case with absolutely positioned object

Comment 82

[karnaze (gone)]

Attachment: test case with absolutely positioned object

Comment 83

[karnaze (gone)]

Attachment: test case with absolutely positioned div containing object

Comment 84

[karnaze (gone)]

Attachment: revised patch to handle unequal top/bottom margins and absolute positioning

Comment 85

[Peter Lubczynski]

Comment on attachment 97654 [details] [diff] [review]
revised patch to handle unequal top/bottom margins and absolute positioning

r=peterl

Comment 86

[kinmoz]

Comment on attachment 97654 [details] [diff] [review]
revised patch to handle unequal top/bottom margins and absolute positioning

sr=[e-mail] ... we really need some generalization so we don't have to
reference table cells directly.

Comment 87

[karnaze (gone)]

The patch is in.

Comment 88

[serge (gone)]

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

Comment 89

[Chris Petersen]

Verified in the OS X (2002-09-09-08) , Linux (2002-09-09-08) , Windows ME
(2002-09-09-08).

Comment 90

[Boris Zbarsky [:bzbarsky]]

This caused crash bug 191246.  Can someone please explain to me what the patch
that was checked in was attempting to do?  I have not been able to find anything
so far that it would be doing correctly....

Comment 91

[Mike Cowperthwaite]

I'd like to reopen this bug, please.  Between released builds 1.1 and 1.2.1,
percentage height within an absolute (or fixed) <DIV> takes the percentage of
the entire window.  Widths work as expected.
The test case #97647 above (test0.html, 
http://bugzilla.mozilla.org/attachment.cgi?id=97647&action=view)
used 100% for heights and so hid the bug.  I'll attach a variant on that which
will make the problem clear:  the applet is given 100% by 100%, and should be
surrounded by the containing DIV's border.

Comment 92

[Mike Cowperthwaite]

Attachment: Example of hosed % heights in absolute DIV

Absolute <DIV> positioned in page, has large border.  Contains a Java <OBJECT>
with 100% x 100% dimensions -- should fill the DIV entirely, and so be
surrounded by border.  In Mozilla 1.2.1 (Win32) the applet's 100% HEIGHT
is calculated against the viewport rather than the DIV; WIDTH is handled
correctly.

Comment 93

[Boris Zbarsky [:bzbarsky]]

Yeah, see my comments in bug 191246 on how this patch is completely screwed up....

Comment 94

[Mike Cowperthwaite]

Sorry, I just reread my comment #91 and realized it was somewhat incoherent.

I meant that the change I'm complaining about occurred between 1.1 and 1.2.1.
The regression is particularly noticeable because this same symptom was fixed
between 0.9.9 and 1.0, and I have a page that uses a %-sized <APPLET> within a
fixed <DIV>.

And when I said the previously submitted test case "used 100%" I meant that
the absolute <DIV> in that example was given a height of 100%, as was the
contained <APPLET>, so the symptom was not visible.

Comment 95

[Boris Zbarsky [:bzbarsky]]

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

Comment 96

[Mike Cowperthwaite]

Is asking for this bug to be reopened (viz. my report in comment #91) the
wrong thing to do?  Should I open a new bug for this issue instead?  (Or has
one been opened that I haven't been able to find?)
Bug 191246 is apparently related, but doesn't address the broken height-setting
behavior.

Comment 97

[Boris Zbarsky [:bzbarsky]]

Mike, the fix for bug 191246 will address the broken height setting behavior,
trust me (since it will have to pass all testcases in this bug).

Comment 98

[Boris Zbarsky [:bzbarsky]]

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