JPG image is not shown with following code snippet. When the height="75%" attribute is removed the image is shown. This happens in Safari 2.0.4 (419.3) and the latest nightly build of webkit. In Firefox 2.0.0.4 the image is shown. !DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN"> <html> <head> </head> <body> <table cellpadding="0" cellspacing="0" border="0" width="100%"> <tr> <td> <img src="Beeldcrypto.jpg" alt="crypto" width="75%" height="75%"> </td> </tr> </table> </body> </html>
Created attachment 16518 [details] code snippet
Created attachment 16519 [details] JPG file
Created attachment 16530 [details] Online test case
Confirmed with a local debug build of WebKit r25955 with Safari 3 Public Beta v. 3.0.3 (522.12.1) on Mac OS X 10.4.10 (8R218). As Comment #0 mentions, also occurs with Safari 2.0.4 (419.3) with original WebKit.
Created attachment 16536 [details] Hack v1 This is a hack that will allow the image to be display correctly within the table (e.g., it displays the same as if the table were removed from the test). I'm setting the review flag to see if this is the correct approach. Note that because of what isHeightSpecified() checks, this change in RenderImage::calcReplacedHeight(): + if (isHeightSpecified() && style()->height().type() != Percent) could really just be: + if (style()->height().type() == Fixed) If this is not the correct approach, then I'd guess that RenderBox::calcReplacedHeightUsing() needs to be taught how to use instrinsicSize() for <img> tags. (Am I in the ballpark here? :) Layout tests running now with this change to check for any failures.
Comment on attachment 16536 [details] Hack v1 (In reply to comment #5) > Layout tests running now with this change to check for any failures. These tests failed, so this is obviously not the correct approach: fast/replaced/absolute-position-percentage-height.html fast/replaced/replaced-child-of-absolute-with-auto-height.html fast/replaced/width100percent-image.html tables/mozilla_expected_failures/bugs/bug85016.html tables/mozilla_expected_failures/bugs/bug137388-2.html
I believe this bug may be generalized to rendering replaced elements with a percentage height within a table cell with auto height (e.g., no height specified). The default (intrinsic) size of a replaced element is 300px wide by 150px high. Thus, this example should render a green 300x112 canvas element: <table><tr><td> <canvas style="background-color: #00ff00; height: 75%;"></canvas> </td></tr></table> On a local debug build of WebKit r26074 with Safari 3 Public Beta v. 3.0.3, this renders a 300x0 canvas element (as verified by the Web Inspector). On Opera 9.22, this renders as a 300x1 canvas element. On Firefox 2.0.0.7, this renders a 300x112 canvas element, but the containing table cell is 300x150 (instead of 300x112). On FIrefox 3.0a9pre (07-Oct-2007), this renders as expected with a 300x112 canvas element. Similar, this example referencing a 600x718 image should render as 450x538 (scaled by 75% proportionally): <table><tr><td> <img src="http://bugs.webkit.org/attachment.cgi?id=16519" alt="crypto" height="75%"> </td></tr></table> On a local debug build of WebKit r26074 with Safari 3 Public Beta v. 3.0.3, this renders as a 0x0 image element (as verified by the Web Inspector). On Opera 9.22, this renders as a 600x718 image element. On Firefox 2.0.0.7, this renders a 450x538 image element, but the containing table cell is 600x718 (instead of 450x538). On FIrefox 3.0a9pre (07-Oct-2007), this renders as a 600x718 image element.
(In reply to comment #7) > The default (intrinsic) size of a replaced element is 300px wide by 150px high. > Thus, this example should render a green 300x112 canvas element: > > <table><tr><td> > <canvas style="background-color: #00ff00; height: 75%;"></canvas> > </td></tr></table> > > On Firefox 2.0.0.7, this renders a 300x112 canvas element, but the containing > table cell is 300x150 (instead of 300x112). > On FIrefox 3.0a9pre (07-Oct-2007), this renders as expected with a 300x112 > canvas element. Actually, on Firefox 3.0a9pre, this is a 300x150 canvas element.
Created attachment 16582 [details] Patch v1 Proposed patch. I think I understand what's going on here (see Comment #7 and WebCore/ChangeLog entry), but I don't have MSIE 6/7 available to test its behavior compared to Safari's new behavior. All layout tests pass that were passing before. A ChangeLog entry, a new test, and updated test results are included in the patch. Hyatt or Mitz should definitely review this.
(In reply to comment #9) > Safari's new behavior. WebKit's new behavior.
(In reply to comment #9) > Created an attachment (id=16582) [edit] > Patch v1 Urgh. LayoutTests changes that may need to be fixed: - Results for LayoutTests/fast/replaced/table-percent-height.html were placed in LayoutTests/platform/mac, but should be platform-independent. (How do I check or generate results for Qt?) - Need updated results for: * LayoutTests/platform/qt/fast/replaced/width100percent-image-expected.txt * LayoutTests/platform/qt/tables/mozilla_expected_failures/bugs/bug137388-1-expected.txt * LayoutTests/platform/qt/tables/mozilla_expected_failures/bugs/bug137388-2-expected.txt
What about form controls.... they need to be tested also (even though they use a different code path).
Comment on attachment 16582 [details] Patch v1 The changes to fast/encoding/char-encoding.html seem unrelated. Please move those to a separate patch. Please also fix the test results as you described. Otherwise, r=me. I think hyatt's concern about form controls can be adressed in a separate patch, since this patch would not regress their behavior and does fix images. r=me
Comment on attachment 16582 [details] Patch v1 Changing mjs' review+ flag to review- since: - I moved fast/encoding/char-encoding.html to a different patch (which already landed). - I want to write additional test cases for this patch (in separate test files per Comment #12). - I need to update the test results per Comment #11.
Created attachment 18137 [details] Patch v2 Changes since Patch v1: - Added tests to LayoutTests/fast/replaced/table-percent-height.html: embed, object, button, input (button, checkbox, file, image, password, radio, reset, submit, text), isindex, select, and textarea. Note that with this patch results changed for: canvas, embed, img, object, and input type="image". - Removed changes to LayoutTests/fast/encoding/char-encoding.html and LayoutTests/fast/js/resources/js-test-post-function.js. - Regenerated pixel test results.
Comment on attachment 18137 [details] Patch v2 I wonder why you went through all the trouble of making a text-only test that uses getComputedStyle. Seems like the direct approach (using the render tree dump) would work just as well.
(In reply to comment #16) > (From update of attachment 18137 [details] [edit]) > I wonder why you went through all the trouble of making a text-only test that > uses getComputedStyle. Seems like the direct approach (using the render tree > dump) would work just as well. One less pixel test. Can you dump the render tree without creating pixel test results?
Comment on attachment 18137 [details] Patch v2 I think this would probably read better if the max() expression was just inside the calcValue -- or maybe not, since there would be no place for the comment. r=me
Committed revision 29039.
*** Bug 5789 has been marked as a duplicate of this bug. ***
The fix for this issue renders the replaced elements inside the table cell properly. But now the table cell width is not getting set properly thereby causing an overlap with its adjacent table cell. New bug added for the same: https://bugs.webkit.org/show_bug.cgi?id=81084