Closed Bug 156731 Opened 22 years ago Closed 22 years ago

Displayed image truncated, correct after reload

Categories

(Core :: Layout, defect, P3)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Future

People

(Reporter: jhg, Assigned: jerry.tan)

References

(
URL
)

Details

(Keywords: testcase)

Attachments

(7 files, 5 obsolete files)

Incorrect display on first load
(deleted), image/gif
Details
Correct display after page reload
(deleted), image/gif
Details
Adding another test case and variation on this bug.
(deleted), text/html
Details
simple testcase
(deleted), text/html
Details
simple testcase II
(deleted), text/html
Details
simple testcase III
(deleted), text/html
Details
patch 0.14
(deleted), patch
karnaze
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
Go to the URL given above. There is an image of a light bulb to the right of the page. When the page first loads, the image is truncated on the right (see attachment ssage1.gif). Press reload and the image is displayed correctly (ssage2.gif). Browser build id 2002053012
Attached image Incorrect display on first load (deleted) —
Attached image Correct display after page reload (deleted) —
Confirming with build 2002070904 WinXP
Attached file Testcase (obsolete) (deleted) —
-> HTMLTables
Assignee: attinasi → karnaze
Component: Layout → HTMLTables
Keywords: testcase
QA Contact: petersen → amar
Status: UNCONFIRMED → NEW
Ever confirmed: true
During the incremental reflow when the image is loaded, the image's max element size (me) of 9000 is not being reflected in the containing div's me. ->layout block 01AED6CC r=1 a=9180,UC c=8940,UC cnt=52 tblO 01AED9DC r=1 a=8940,UC c=0,0 cnt=53 tbl 01AEDBD4 r=1 a=8940,UC c=11745,UC cnt=54 rowG 01AEDD8C r=1 a=11745,UC c=11745,UC cnt=55 row 01AEDF18 r=1 a=11745,UC c=11745,UC cnt=56 cell 01AE90BC r=1 a=450,UC c=390,UC cnt=57 block 01AE911C r=1 a=390,UC c=390,UC cnt=58 block 01AE93A8 r=1 a=390,UC c=390,UC cnt=59 img 01AE96C4 r=4 a=UC,UC c=UC,UC cnt=61 img 01AE96C4 d=9000,870 me=9000 img 01AE96C4 r=2 a=390,UC c=UC,UC cnt=64 img 01AE96C4 d=9000,870 block 01AE93A8 d=420,968 me=420 m=9030 VALUE 968 is not a whole pixel block 01AE911C d=9015,968 me=390 m=9000 VALUE 968 is not a whole pixel cell 01AE90BC d=9075,1035 me=450 m=9060 row 01AEDF18 d=11745,1035 rowG 01AEDD8C d=11745,1035 tbl 01AEDBD4 d=11775,1125 tblO 01AED9DC d=11775,1125 block 01AED6CC d=8940,1125
Assignee: karnaze → attinasi
Component: HTMLTables → Layout
QA Contact: amar → petersen
Confirming build 0000000000 redhat 7.3 2.4.18
OS: Windows 2000 → All
Hardware: PC → All
QA Contact: petersen → amar
Priority: -- → P3
Target Milestone: --- → Future
Adding another test case. Changing height of myclass1 from % to px causes myclass2's image to display correctly without reload.
assign it to me.
Assignee: attinasi → jerry.tan
Blocks: 166758
Attached file simple testcase (deleted) —
only one image inside body, with percent css height and width.
when shift-reload simple testtcase, it will do reflow only once, for initial. during it, it has no internal image size. but it has percent css width and height value, height is unconstrained value, width isnot. so during GetDesiredSize(), it will set height as 0. when reload simple testcase, during reflow, it has internal image size(intrinsicHeight,intrinsicWidth), although css style height is also unconstrained value, but during GetDesiredSize(), it will set height as (intrinsicHeight * newWidth) / intrinsicWidth. so that is why reload and shift-reload give different render result for image with percent css style height. compare simple testcase with another simple testcase 2 <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/1999/REC-html401-19991224/loose.dtd"> <html> <body> <img src="mozilla-banner.gif"/> </body> </html> when shift-load simple testcase2, it will do initial reflow, then do another resize reflow(during nsImageFrame::OnStartContainer), to set img frame size. so my idea to fix it is that: force shift-reload for that image to do resize reflow after it get its intrinsicHeight and intrinsicWidth.
Status: NEW → ASSIGNED
Attached patch patch 0.1 (WIP) (obsolete) (deleted) — — Splinter Review
I think this bug could be divided into two seperate bugs. one is with simple testcase #100973, image with percent css height value another is with Testcase #90834, image without css height and width value, but inside tables. as to bug one, the root reason can refer to my comment #11, as to bug two, I think the reason is that when do increase reflow to get image's mIntrinsicSize, it will set ancestor table cell's width, but it doesnot recaculate the whole table cells width after it.
*** Bug 172723 has been marked as a duplicate of this bug. ***
Attached patch patch 0.11 (obsolete) (deleted) — — Splinter Review
Attachment #101048 - Attachment is obsolete: true
patch 0.11 is for simple testcase.
Attached file simple testcase II (deleted) —
another testcase, image with percent height inside div which is inside table.
You could avoid the local variable and have fewer lines with the following: return ((heightUnit == eStyleUnit_Percent) && (aReflowState.mComputedHeight == NS_UNCONSTRAINEDSIZE)) ? PR_FALSE : HaveFixedSize(*(aReflowState.mStylePosition)); Can mComputedWidth ever be NS_UNCONSTRAINEDSIZE? If not, r=karnaze.
Attached file simple testcase III (deleted) —
simple testcase III, img inside one table. mComputedWidth can also be NS_UNCONSTRAINEDSIZE.
Attached patch patch 0.12 (obsolete) (deleted) — — Splinter Review
patch 0.12, for three simple testcase. when imgae inside table, in table's first reflow, image's reflowState.mComputedWidth is NS_UNCONSTRAINEDSIZE, then in table's second reflow, image's reflowState.mComputedWidth is 0. so make patch 0.12 like this.
Attachment #102306 - Attachment is obsolete: true
r=karnaze. Just a minor nit to align parens between lines, break up the long line. There is also an unwritten coding convention to put a constant on the left side of "==" (e.g. eStyleUnit_Percent == heightUnit, etc.) to avoid the mistake of using "=" instead, even though it is less readable; return (((heightUnit == eStyleUnit_Percent) && (aReflowState.mComputedHeight == NS_UNCONSTRAINEDSIZE)) || ((widthUnit == eStyleUnit_Percent) && (aReflowState.mComputedWidth == NS_UNCONSTRAINEDSIZE || aReflowState.mComputedWidth == 0)))
Attached patch patch 0.13 (obsolete) (deleted) — — Splinter Review
Attachment #102771 - Attachment is obsolete: true
Comment on attachment 102891 [details] [diff] [review] patch 0.13 r=karnaze
Attachment #102891 - Flags: review+
bz, can you give sr for patch 0.13? after it, I will concentrate on Testcase #90834, image without css height and width value, but inside tables.
Comment on attachment 102891 [details] [diff] [review] patch 0.13 Add a nice comment to the code explaining the logic (including the weird-looking |0 == mComputedWidth| check)? Just give a high-level overview of the problem; no need to go into details. Don't overparenthesize the boolean expression. "&&" has higher precedence than "==" (but keep the parens around the "||" stuff, even though those are technically not needed -- they improve readability). With those two changes, sr=bzbarsky
Attached patch patch 0.14 (deleted) — — Splinter Review
Attachment #102891 - Attachment is obsolete: true
Comment on attachment 102908 [details] [diff] [review] patch 0.14 > + // when a image "an image" > + // it need "it needs". Same change in one other place in the comment. No need to attach a new patch for those spelling changes. sr=bzbarsky
Attachment #102908 - Flags: superreview+
Comment on attachment 102908 [details] [diff] [review] patch 0.14 r=karnaze. Jerry, it would look better if your comment lines were about the same length.
Attachment #102908 - Flags: review+
to make things simple, I will file another bug for Testcase 90834, and close this bug.
Comment on attachment 90834 [details] Testcase file new bug 174665 base on this testcase, so make it obsolete here.
Attachment #90834 - Attachment is obsolete: true
fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment on attachment 102908 [details] [diff] [review] patch 0.14 >+ return ((eStyleUnit_Percent == heightUnit && NS_UNCONSTRAINEDSIZE == aReflowState.mComputedHeight) || >+ (eStyleUnit_Percent == widthUnit && (NS_UNCONSTRAINEDSIZE == aReflowState.mComputedWidth || >+ 0 == aReflowState.mComputedWidth))) >+ ? PR_FALSE >+ : HaveFixedSize(*(aReflowState.mStylePosition)); A little after-the-fact thought: "x ? PR_FALSE : y" can also, when y is a boolean, be written as "!x && y", and I think is usually clearer that way. Also, the "0 == aReflowState.mComputedWidth" bit seems mis-indented.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: