276286 - scrollbox reports wrong maximumWidth

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, defect)

Description

[crisp]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a6) Gecko/20041228

In the page at http://allcrispy.com/meuk/moz_overflow_bug.html an element with
overflow:auto is set within a table-cell context. When through javascript a
style-element (in this testcase the bottom-padding) is set or changed the width
of the element collapses to zero width. This worked fine in Gecko 1.7.x.
This testcase is a simplification of the code used at
http://gathering.tweakers.net to expand and collapse code previews using javascript.

Reproducible: Always

Steps to Reproduce:
1. Open the testpage in browser with Gecko engine 1.8.x

Actual Results:  
The element used in this case (a div) has zero width after the javascript
execution - only the red border remains visible

Expected Results:  
The content should be visible within the element; the original width of the
layed out element should not have been effected by the setting of the padding
through javascript.

Comment 1

[crisp]

Attachment: Testcase attached for convenience

Comment 2

[Bernd]

Hmm if one looks at the incremental reflow there are two things that go wrong
1. we target a dirty reflow at the inner cell block
2. the xul box reports a wrong maximumWidth (me) it should be the size of the
content 552 in this instance.

 cell 037D84B0 r=1 a=576,UC c=552,UC cnt=347
  block 037D8510 r=1 incr. (Dirty) a=552,UC c=552,UC cnt=348 <<<<< 1.)
   scroll 037D8810 r=4 a=552,UC c=528,UC cnt=349
   scroll 037D8810 r=4 a=552,UC c=528,UC cnt=350
    area 037D87BC r=3 a=528,UC c=528,UC cnt=351
     text 037DD400 r=3 a=528,UC c=UC,UC cnt=352
     text 037DD400 d=528,228
    area 037D87BC d=528,240
    area 037D87BC r=2 a=528,UC c=528,240 cnt=353
    area 037D87BC d=528,240 me=528
   scroll 037D8810 d=552,264 me=552 m=0
   scroll 037D8810 d=552,264 me=24 m=0 <<<<< 2.)
  block 037D8510 d=552,264 me=24 m=0
 cell 037D84B0 d=576,288 me=48 m=48

Comment 3

[Bernd]

Boris: How can one debug such a wrong target frame for incr. reflow issue?

Comment 4

[Bernd]

Attachment: update mMaximumWidth if requested

Comment 5

[Bernd]

   scroll 037D8810 d=552,264 me=552 m=0
   scroll 037D8810 d=552,264 me=24 m=0 <<<<< 2.)

this should be more 

   scroll 037D8810 d=552,264 me=552 m=0  <<<<< 2.)
   scroll 037D8810 d=552,264 me=24 m=0

and the maximumWidth is of course m not me (what a shame, I need more coffee in
the morning)

Comment 6

[Boris Zbarsky [:bzbarsky]]

Normally, when style is changed on a node, we should target a style-change
reflow at the node....

If we're actually targeting something else with a dirty reflow, I'd start by
putting a breakpoint in nsPresShell::AppendReflowCommand and seeing what the
callstack for the dirty reflow command is.

Comment 7

[Bernd]

Boris thanks, the evil code is at
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/xul/base/src/nsBox.cpp&mark=333-339
instead of posting a incr. style change reflow boxes are directly reflown inside
nsCSSFrameConstructor::StyleChangeReflow. This thing will certainly cause
problems if somebody places a normal frame inside a xul frame and causes a style
change reflow on the xul box.

Comment 8

[Boris Zbarsky [:bzbarsky]]

So we basically want to switch to the commented-out code there, no?

Past that, though, it looks like the style-change coalescing mechanism is just
wrong -- it will suppress style-change reflows on boxes whose parents already
have a style-change reflow targeted at them.  Or am I missing something?  If I'm
right, we should remove this coalescing altogether, I think, because coalesing
style change reflows targeted at the same frame is already handled by the reflow
path stuff...

Comment 9

[Bernd]

I have to take back my comment about the problems. The strange reflow mechanism
will cause a style change reflow on the children of the xul box which is the
right thing.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 169802 [details] [diff] [review]
update mMaximumWidth if requested

r+sr=bzbarsky

Comment 11

[Bernd]

I dont think that it should block 1.8.6

Comment 12

[Bernd]

If somebody thinks this should make it for 1.8.6a feel free to check it in as I
will habe limited time for it over the week.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 169802 [details] [diff] [review]
update mMaximumWidth if requested

I think this is safe enough for 1.8a6, and this fixes bug 261376 and probably a
whole slew of other bugs we have...

Comment 14

[Asa Dotzler [:asa]]

Comment on attachment 169802 [details] [diff] [review]
update mMaximumWidth if requested

a=asa (on behalf of drivers) for checkin to Mozilla 1.8a6.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Adding more bugs fixed by this patch to the dependency list.  Ccing Martijn in
case he has time to verify the fix once this goes in...

Comment 16

[Boris Zbarsky [:bzbarsky]]

Fix checked in for 1.8a6 on bernd's behalf.