Created attachment 97388 [details] test case After r88913, I'm seeing a regression where an <object> with height=100% no longer having a height of 100%. Instead, the height is "100%", but it's computed to be 150 px. I tested with the attached html file. You may also need a PDF viewer and a dummy test.pdf file to actually display the object. In the attached screenshot, the left copy of Chromium is with webkit r88913. The one on the right is 13.x with an older version of webkit.
Created attachment 97389 [details] screenshot of the problem
Sounds pretty bad. Niko, how soon can you fix this?
Excellent catch! I feared we had no layout test coverage for object+PDF. How can that be possible _at all_? Who landed PDFDocument w/o proper test... Enough of the moaning, I'll take care to fix this, and add decent test coverage for it, ok?
Some background, for those who wonder what has changed: int RenderReplaced::computeReplacedLogicalHeight() const { // 10.5 Content height: the 'height' property: http://www.w3.org/TR/CSS21/visudet.html#propdef-height // If the height of the containing block is not specified explicitly (i.e., it depends on // content height), and this element is not absolutely positioned, the value computes to 'auto'. bool heightIsAuto = style()->logicalHeight().isAuto(); if (!document()->inQuirksMode() && !isPositioned() && style()->logicalHeight().isPercent()) { if (RenderObject* containingBlock = this->containingBlock()) { while (containingBlock->isAnonymous()) containingBlock = containingBlock->containingBlock(); heightIsAuto = !containingBlock->style()->logicalHeight().isSpecified(); } } ... This behavior is correct according to 2.1. Though as you can see above I had to disable this logic for two cases: 1) document is in quirks mode: don't apply the height="xx" -> height="auto" computation. 2) containingBlock is anonymous, the spec doesn't say anything about this, and in order to not break table/mozilla/ it had to be done this way. Hm, do we need more exceptions for PDF? I'm not sure. Maybe Dave can comment as well? I'll also CC Simon Fraser, maybe he's got an opinion. Side note: removing that heightIsAuto correction completely, will break some of the new CSS 2.1 testcases that I imported... *sigh*.
Hi, Looking at the test with Chrome's Developer Tools, object is actually 100% of the height of it's parent tag, body. I'm not sure, but isn't this the expected behaviour? body defaults to auto height, which is computed to be about 155px. A quick fix is either to: 1. Use position:absolute to force to be 100% of window height, not body height 2. Use height:100% on html,body in the actual html file rather than depending that body will always be the size of the window.
Yes, it is possible to modify the html / css to do display correctly. The question is, what is the preferred behavior for the test case as is? I have no idea.
(In reply to comment #5) > Hi, > Looking at the test with Chrome's Developer Tools, object is actually 100% of the height of it's parent tag, body. I'm not sure, but isn't this the expected behaviour? body defaults to auto height, which is computed to be about 155px. > A quick fix is either to: > 1. Use position:absolute to force to be 100% of window height, not body height > 2. Use height:100% on html,body > in the actual html file rather than depending that body will always be the size of the window. We should compare our behaviour for that testcase with other browsers. Did anyone look at IE9/Opera/FF and whether they pass the testcase? As I said before, the 100% -> 150px is correct here, according to CSS 2.1.
This seems to affect iframes with height 100% too (test case at http://beta.everlong.org/iframe2/, reported on the Chromium side as http://crbug.com/86489)
See also http://code.google.com/p/chromium/issues/detail?id=87260
Would be useful if the title said *when* the computed height is wrong.
For the IFrame problem, I can tell it gets the wrong computed height even if its parent is not body. In the aforementioned testcase (http://beta.everlong.org/iframe2/), we have the following DOM tree : html body div.iframe iframe div.iframe is absoluted positioned, with top = right = bottom = left = 0px. In my setup it gets computed height = 728px. iframe has height:100% and gets height = 150px, which is completely wrong. As I said in my original Chromium bug report, the only browsers doing this are IE7 and older. That's painful.
(In reply to comment #11) > For the IFrame problem, I can tell it gets the wrong computed height even if its parent is not body. > > In the aforementioned testcase (http://beta.everlong.org/iframe2/), we have the following DOM tree : > > html > body > div.iframe > iframe > > div.iframe is absoluted positioned, with top = right = bottom = left = 0px. > In my setup it gets computed height = 728px. > > iframe has height:100% and gets height = 150px, which is completely wrong. > > As I said in my original Chromium bug report, the only browsers doing this are IE7 and older. That's painful. Hi Julien, sorry I forgot to mention here that in bug 64059 I've discussed the problem, and how to fix it. I'll come to it soon! Cheers, Niko
(In reply to comment #0) > Created an attachment (id=97388) [details] > test case > > After r88913, I'm seeing a regression where an <object> with height=100% no longer having a height of 100%. Instead, the height is "100%", but it's computed to be 150 px. > > I tested with the attached html file. You may also need a PDF viewer and a dummy test.pdf file to actually display the object. > > In the attached screenshot, the left copy of Chromium is with webkit r88913. The one on the right is 13.x with an older version of webkit. Sorry, I can't get your testcase running. Whatever pdf I use, it doesn't show up -- doesn't Safari support pdf-in-<object> ?? Is it chromium specific? If so, please reduce to a testcase that works in Safari.
I can't open the original testcase with Safari, it doesn't seem to be able to embed pdf this way, I didn't investigate further. Though the problem is identical to bug 64059, and should be fixed with it as well.
(In reply to comment #13) > Sorry, I can't get your testcase running. Whatever pdf I use, it doesn't show up -- doesn't Safari support pdf-in-<object> ?? Is it chromium specific? If so, please reduce to a testcase that works in Safari. The pdf plugin is Chromium specific. I think you can get the same result if you replace the <object> with: <embed src="test.swf" type="application/x-shockwave-flash" width="100%" height="100%"></embed>
Thanks a lot! My feeling was right: the underlying bug is the same. Marking as duplicate of bug 64059, whose patch fixes this.(In reply to comment #15) > (In reply to comment #13) > > Sorry, I can't get your testcase running. Whatever pdf I use, it doesn't show up -- doesn't Safari support pdf-in-<object> ?? Is it chromium specific? If so, please reduce to a testcase that works in Safari. > > The pdf plugin is Chromium specific. I think you can get the same result if you replace the <object> with: > > <embed src="test.swf" type="application/x-shockwave-flash" width="100%" height="100%"></embed> Thank you, I can confirm the underlying issue is the same as 64059. Marking as dup *** This bug has been marked as a duplicate of bug 64059 ***
(In reply to comment #16) The fix landed in r91242. Please test and comment whether the original bug with the PDF in Chromium is gone as well.