Note: There are a few cases of duplicates in user autocompletion which are being worked on.

[FIX] incremental reflow goes wrong when block has spaces around it




Layout: Block and Inline
14 years ago
14 years ago


(Reporter: Hixie (not reading bugmail), Assigned: Mats Palmgren (vacation - back in August))


({css1, testcase})

css1, testcase

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [patch], URL)


(7 attachments)



14 years ago
  div { border: 1px solid; width: 25em; margin: 2em auto; }
  <table><tr><td><div></div><td valign="bottom"> <div></div> </td></tr></table>

  The spaces around the <div> in the second cell are required to show the bug.

  Two adjacent lines.

  The second line is first drawn too high. A reflow (font zoom change, e.g.) 
  will move the second line down.

Comment 1

14 years ago
Created attachment 125435 [details]
dynamic testcase showing the correct behaviour

Comment 2

14 years ago
Created attachment 125437 [details]
reflow log

Comment 3

14 years ago
the second (resize) reflow
         cell 0266F168 r=2 a=6060,UC c=6030,UC cnt=878 
          block 0266F23C r=2 a=6030,UC c=6030,UC cnt=879 
           text 0266F2E8 r=2 a=6030,UC c=UC,UC cnt=880 
           text 0266F2E8 d=0,0 
           block 0266F39C r=2 a=6030,UC c=6000,UC cnt=881 
           block 0266F39C d=6030,30 
          block 0266F23C d=6030,1470 
         cell 0266F168 d=6060,1500 

and the style change reflow 
         cell 0266F168 r=3 a=6060,UC c=6030,UC cnt=907 
          block 0266F23C r=3 a=6030,UC c=6030,UC cnt=908 
           text 0266F2E8 r=3 a=UC,UC c=UC,UC cnt=909 
           text 0266F2E8 d=0,0 me=0 
           text 0266F2E8 r=3 a=6030,UC c=UC,UC cnt=910 
           text 0266F2E8 d=0,0 
           block 0266F39C r=3 a=6030,UC c=6000,UC cnt=911 
           block 0266F39C d=6030,30 me=6030 m=15 
           text 0266F3F0 r=3 a=UC,UC c=UC,UC cnt=912 
           text 0266F3F0 d=0,0 me=0 
           text 0266F3F0 r=3 a=6030,UC c=UC,UC cnt=913 
           text 0266F3F0 d=0,0 
          block 0266F23C d=6030,990 me=6030 m=15 
         cell 0266F168 d=6060,1020 me=6060 m=6060 

The block is reporting a too large height 1470 instead of 990 ( just a blind
guess 1470 = 990 + 990/2)
This is in my oppinion a block bug
Assignee: table → block-and-inline
Component: Layout: Tables → Layout: Block & Inline
QA Contact: madhur → ian
This seems to be related to the 'auto' side margins.
Component: Layout: Block & Inline → Layout: Tables
QA Contact: ian → madhur
Component: Layout: Tables → Layout: Block & Inline
QA Contact: madhur → ian
Created attachment 125444 [details]
simpler testcase

Only one piece of whitespace (plus borders around the table cells).
Attachment #125444 - Attachment is patch: false
Attachment #125444 - Attachment mime type: text/plain → text/html; charset=iso-8859-1

Comment 6

14 years ago
*** Bug 214394 has been marked as a duplicate of this bug. ***

Comment 7

14 years ago
From bug 214394:

The rendering of the testcase changed between 2001-10-23-08 and 
2001-10-26-08.  Of the checkins in that range, the one for bug 86947 arouses the
most suspicion in me, followed by deCOMtamination of nsSpaceManager and
deCOMtamination of nsIFrame that hyatt landed...

Comment 8

14 years ago
Created attachment 128835 [details]
This may also be this same bug....

In this testcase, the bottom margin of the <div> is absent for some reason. 
Making the left/right margins not auto fixes it....

Comment 9

14 years ago
seen on linux, -> all/all
OS: Windows 2000 → All
Hardware: PC → All
FWIW, it seems this bug only occurs within tables.
Keywords: testcase
Taking this since it's unassigned and I have a fix...
Assignee: block-and-inline → mats.palmgren
Keywords: css1
Summary: incremental reflow goes wrong when block has spaces around it → [FIX] incremental reflow goes wrong when block has spaces around it
Whiteboard: [patch]
Created attachment 129993 [details] [diff] [review]
Patch rev. 1

The bug is in nsHTMLReflowState::ComputeMargin() - when the containing block
is NS_UNCONSTRAINEDSIZE we simply ignore any specified top/bottom margin and
set the computed value to zero.  I have just removed this bit and let
ComputeHorizontalValue() deal with it (since it can do that now).
Created attachment 129994 [details] [diff] [review]
Patch rev. 1  (diff -uw for easier review)
Attachment #129993 - Flags: review?(dbaron)
Attachment #129993 - Flags: superreview+
Attachment #129993 - Flags: review?(dbaron)
Attachment #129993 - Flags: review+

Comment 14

14 years ago
Mats, are you confident enough in this fix to go for getting it into 1.5?  If 
so, please request approval?
Comment on attachment 129993 [details] [diff] [review]
Patch rev. 1

I am very confident that this patch does the right thing.
Attachment #129993 - Flags: approval1.5b?

Comment 16

14 years ago
Comment on attachment 129993 [details] [diff] [review]
Patch rev. 1

1.5b shipped. moving request forward.
Attachment #129993 - Flags: approval1.5b? → approval1.5?

Comment 17

14 years ago
Mats, is this likely to break backward compatibility or cause regressions out on
the web?
No, it only occurs when left or right margin has the specified value 'auto' and
and a specified non-zero top or bottom margin (which we currently ignore).
This combination is rarely used by CSS authors.
(The initial margin value is zero)

It does affect <hr> though (after bug 38370 was fixed) since it now uses this
combination of margin values.  I think it's more likely that there will be
regressions from bug 38370 if this patch is NOT checked in.
Created attachment 130871 [details]
Screenshot of 1st testcase in bug 119606 (with the patch applied)

Funny, I just stumbled into a regression from bug 38370.  See the first
testcase in bug 119606 and compare the last two columns with this screenshot.
I think we should get this in for 1.5.

Comment 21

14 years ago
Comment on attachment 129993 [details] [diff] [review]
Patch rev. 1

a=asa (on behalf of drivers) for checkin to Mozilla 1.5
Attachment #129993 - Flags: approval1.5? → approval1.5+
David / Boris - could you check this in for me?  Thanks.

Comment 23

14 years ago
I have no CVS access till I get back.  biesi, could you do this if dbaron
doesn't get to it soon?
Fix checked in to trunk, 2003-09-05 21:01 -0700.
Last Resolved: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.