12122 - [BLOCKER][DOGFOOD]overflow property within XUL == disappearing content

Status: VERIFIED FIXED

(Core :: XUL, defect, P1)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

DESCRIPTION:
When the overflow property is set to scroll or auto within XUL, it causes the
content within the element with the overflow property to disappear (and draws a
few scrollbars if overflow is scroll).

STEPS TO REPRODUCE:
* download the four files stock-ticker.* in
  http://www.fas.harvard.edu/~dbaron/nstmp/stock-ticker/M9-3/
* Add to the beginning of stock-ticker.css either the rule:
    tbody#stockTB { overflow: auto; }
  or the rule
    tbody#stockTB { overflow: scroll; }
* load stock-ticker.xul in viewer or apprunner or in the sidebar (currently
  does not work from an HTTP URL - see bug 11112.)

ACTUAL RESULTS:  The content within the table body (listing of five stocks with
data) disappears.  If you picked "overflow: scroll" above, then there are
scrollbars on the *top* and right of the area where they should be, except the
one on the right is very short.

EXPECTED RESULTS:  If the panel is displayed in a large window (such as viewer
or apprunner's content window), the stocks should all fit, and if overflow is
scroll (rather than auto) there should be scrollbars.

If the panel is displayed in a small window, such as the sidebar (you need to
edit chrome://sidebar/content/sidebar-browser.rdf to do this), the scrollbars
should allow you to scroll in either case, and the stock data should take up as
much room as it can so that nothing overflows off the edge of the page (kind of
like an IFRAME, but no horizontal scrolling should be needed since the widths
get sized to fit).

BUILDS TESTED:
 * Linux, apprunner, 1999-08-16-08-M9
 * Linux, viewer, 1999-08-16-08-M9

ADDITIONAL INFORMATION:
Being able to use overflow is important for sidebar panels, since they will
often want to have concepts of "navigation" and "data", where the navigation
should always fit even if the sidebar panel is tiny and the data must overflow.

If you want to allow scrolling some other way than the overflow property, that
would be reasonable (but it would make sidebar panel developers learn yet
something else).  However, the overflow property shouldn't cause the disastrous
results it currently causes.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think this bug may be similar to some other bugs I've filed (or at least
things that have bothered me).  The way things seem to work now is that an XUL
box reflows its contents to be "as small as possible".  What I think should
happen is that, after this happens, if the box's size is increased by a "flex"
attribute, the contents should be reflowed again with the new size for the box.
I think I've seen elsewhere (auto margins - bug 9442 ??) that this should
happen.

Comment 2

[Peter Trudelle]

reassigning to evaughan for triage

Comment 3

[Eric Vaughan]

This is fixed with when I turn on gfx scrollbars. I mark fixed once I turn them
on.

Comment 4

[ekrock's old account (dead)]

When is GFX scheduled to be turned on?  Sounds like this bug will be fixed when
that happens, which is great news, but to make sure this bug is prioritized
appropriately, I made the following changes as this is blocking David Baron's
panel development:
1) changed from P3 to P1
2) set target milestone to M10
3) marked [BLOCKER][DOGFOOD] in status whiteboard because it is blocking the
development of panels and preventing us from reaching dogfood for panel
developability
4) added relevant PMs to cc: list

Also note that David's internship ends in late September; if we don't get this
fixed by the end of August at the latest, I doubt we'll be able to have David
finish as much panel demo, sample code, and doc development work as we had
hoped.

Totally understand the pressure everyone's under; just want to make sure this
bug gets the attention it needs so we can move forward and make the panels a
success.

Comment 5

[leger]

If Linux only, please put on [PP] radar

Comment 6

[Eric Vaughan]

Overflow on boxes and tabs work now. But tables are html. Are you able to get
this to work in a regular html table in a html file?

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yes, it works fine within HTML.  Example to be attached.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: example showing it works fine in HTML

Comment 9

[Eric Vaughan]

Attachment: xul example that show scrollbars

Comment 10

[Eric Vaughan]

Your scrollbars don't show up because the second table forces the first table
not to get smaller. If you make the first tables width bigger scrollbars will
appear. See the example enclosed.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The problem was the interaction of XUL boxes with flex with overflow auto on an
element inside them.  Your example doesn't have any boxes.

Reopening.

The original URL was wrong.  It should be
http://www.fas.harvard.edu/~dbaron/nstmp/M9-3/ .  That example only has one
table.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I put the test cases for this bug (revised as described in the original report),
in http://www.fas.harvard.edu/~dbaron/nstmp/M9-3-bug12122/ .  They are now
crashing viewer and apprunner even when viewed locally (1999-09-14-09-M11).

Comment 13

[Peter Trudelle]

Moving [BLOCKER][DOGFOOD]from status whiteboard to summary to get on radar.  Is
this still a problem with GFX scrollbars?

Comment 14

[leger]

From our reading GFX scroll bars take care of this.  Please send bug to
[e-mail] if you feel this should be put back on dogfood radar.  Putting
on [PDT-] radar.

Comment 15

[Eric Vaughan]

This example crashes in tables:

nsTableFrame::ComputeDesiredHeight(nsIPresContext & {...}, const
nsHTMLReflowState & {...}, int 285) line 4201 + 14 bytes
nsTableFrame::ResizeReflowPass2(nsTableFrame * const 0x027aa8a0, nsIPresContext
& {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned
int & 0) line 2618 + 20 bytes
nsTableFrame::Reflow(nsTableFrame * const 0x027aa8a4, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 2318 + 38 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x027aa8a0, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 372 + 28 bytes
nsTableOuterFrame::Reflow(nsTableOuterFrame * const 0x027aa944, nsIPresContext &
{...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned
int & 0) line 902 + 37 bytes
nsBoxFrame::FlowChildAt(nsIFrame * 0x027aa940, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0,
nsCalculatedBoxInfo & {...}, int & 0, nsString & {...}) line 1100
nsBoxFrame::FlowChildren(nsIPresContext & {...}, nsHTMLReflowMetrics & {...},
const nsHTMLReflowState & {...}, unsigned int & 0, nsRect & {...}) line 699
nsBoxFrame::Reflow(nsBoxFrame * const 0x0127fd04, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 534
nsBoxFrame::FlowChildAt(nsIFrame * 0x0127fd00, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0,
nsCalculatedBoxInfo & {...}, int & 0, nsString & {...}) line 1100
nsBoxFrame::FlowChildren(nsIPresContext & {...}, nsHTMLReflowMetrics & {...},
const nsHTMLReflowState & {...}, unsigned int & 0, nsRect & {...}) line 699
nsBoxFrame::Reflow(nsBoxFrame * const 0x012f0434, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 534
nsBoxFrame::FlowChildAt(nsIFrame * 0x012f0430, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0,
nsCalculatedBoxInfo & {...}, int & 0, nsString & {...}) line 1100
nsBoxFrame::FlowChildren(nsIPresContext & {...}, nsHTMLReflowMetrics & {...},
const nsHTMLReflowState & {...}, unsigned int & 0, nsRect & {...}) line 699
nsBoxFrame::Reflow(nsBoxFrame * const 0x023f7294, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 534
nsContainerFrame::ReflowChild(nsIFrame * 0x023f7290, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 372 + 28 bytes
RootFrame::Reflow(RootFrame * const 0x027a8b14, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 330
nsContainerFrame::ReflowChild(nsIFrame * 0x027a8b10, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 372 + 28 bytes
ViewportFrame::Reflow(ViewportFrame * const 0x027a8b84, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 516
PresShell::InitialReflow(PresShell * const 0x027543e0, int 10875, int 8475) line
949

Comment 16

[karnaze (gone)]

Eric, the fix I just checked in for bug 12012 must have fixed this also, since
I'm not seeing the crash.

Comment 17

[Eric Vaughan]

Now this crashes in tables. Its particularly spectacular when gfx scrollbars are
turned on. The problem seems to be because the table casts
the rowGroupFrame to  at nsTableRowGroupFrame*. The only problem is the
rowGroupFrame is a GfxScrollFrame. If I turn off
gfx scrollbars it crashes in ComputeDesiredHeight just like it did before.

nsGfxScrollFrameInner::QueryInterface(nsGfxScrollFrameInner * const 0x02759adc,
const nsID & {...}, void * * 0x00000007) line 228 + 206 bytes
nsTableFrame::ComputeDesiredHeight(nsIPresContext & {...}, const
nsHTMLReflowState & {...}, int 300) line 4163 + 36 bytes
nsTableFrame::ResizeReflowPass2(nsTableFrame * const 0x02759ae0, nsIPresContext
& {...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned
int & 0) line 2588 + 20 bytes
nsTableFrame::Reflow(nsTableFrame * const 0x02759ae4, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 2285 + 38 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x02759ae0, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 378 + 28 bytes
nsTableOuterFrame::Reflow(nsTableOuterFrame * const 0x02759b74, nsIPresContext &
{...}, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned
int & 0) line 907 + 37 bytes
nsBoxFrame::FlowChildAt(nsIFrame * 0x02759b70, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0,
nsCalculatedBoxInfo & {...}, int & 0, nsString & {...}) line 1130
nsBoxFrame::FlowChildren(nsIPresContext & {...}, nsHTMLReflowMetrics & {...},
const nsHTMLReflowState & {...}, unsigned int & 0, nsRect & {...}) line 699
nsBoxFrame::Reflow(nsBoxFrame * const 0x0242395c, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 534
nsBoxFrame::FlowChildAt(nsIFrame * 0x02423958, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0,
nsCalculatedBoxInfo & {...}, int & 0, nsString & {...}) line 1130
nsBoxFrame::FlowChildren(nsIPresContext & {...}, nsHTMLReflowMetrics & {...},
const nsHTMLReflowState & {...}, unsigned int & 0, nsRect & {...}) line 699
nsBoxFrame::Reflow(nsBoxFrame * const 0x02421e44, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 534
nsBoxFrame::FlowChildAt(nsIFrame * 0x02421e40, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0,
nsCalculatedBoxInfo & {...}, int & 0, nsString & {...}) line 1130
nsBoxFrame::FlowChildren(nsIPresContext & {...}, nsHTMLReflowMetrics & {...},
const nsHTMLReflowState & {...}, unsigned int & 0, nsRect & {...}) line 699
nsBoxFrame::Reflow(nsBoxFrame * const 0x0236aeb4, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 534
nsContainerFrame::ReflowChild(nsIFrame * 0x0236aeb0, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 378 + 28 bytes
RootFrame::Reflow(RootFrame * const 0x0225d944, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 330
nsContainerFrame::ReflowChild(nsIFrame * 0x0225d940, nsIPresContext & {...},
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0)
line 378 + 28 bytes
Viewpo

Comment 18

[karnaze (gone)]

It might help if someone told me how to turn on gfx scroll bars, preferably in
Viewer. The scroll bars upon setting style/widget-rendering-mode/gfx don't look
any different than native ones.

Comment 19

[karnaze (gone)]

Eric, with native scroll bars in stock-ticker.xul, I'm seeing a scroll frame
(not scrollable frame) with a display type of NS_STYLE_DISPLAY_TABLE_ROW_GROUP
and with gfx scroll bars, the gfx scroll bar has that display. This has to be
changed because the table code makes assumptions about the frame type based on
the display type. I noticed a lot of new code in nsCSSFrameConstructor and
nsScrollFrame that presumably you wrote, so I'm giving this back to you.

Comment 20

[karnaze (gone)]

Attachment: simpler stock-ticker.xul causing crash (as plain text)

Comment 21

[Peter Trudelle]

mass-moving all m12 bugs to m13

Comment 22

[Eric Vaughan]

The table should not be making assumption about display type. We should have a
interface on the frame that if implemented that can return the scrolled frame
inside. So when a table is looking at one of its rows it should see if it
implements this interface. If it does it should ask the interface for the
scrolled frame. This is very much like nsIScrollableView but at the frame level.
That way the implementation of the the scrolling code can be left as
implementation.

Chris Karnaze wrote:

> I didn't think this discussion necessarily had to do with with scrollable row
> groups, but it might. The table code was crashing because a scroll frame had
the
> display type of row group which is clearly wrong. Recall that the table code
> casts frames based on display type. So, what has changed is that scroll frames
> have the wrong display type and the question is "what display type should they
> have".

There is no special CSS display type for something that is scrolled. Therefore,
the
scroll frame will have a display type of row group. And that's not wrong, that's
the
way it is

A large part of the motivation for GetFrameType() was so that we could check
what
kind of frame we actually have. If you want to know whether it's a scroll frame,
then you should use GetFrameType()

-Troy

Comment 23

[troy]

I want to avoid adding extra interfaces to frames unless we absolutely need to.
It costs us 4 bytes overhead per frame instance for each interface the frame
class implements.

The frame class should be sufficient to tell whether we have a scroll frame

That said, I think the equivalent of nsIScrollableView at the frame level makes
sense, and since only the scroll frame (or gfx scroll frame or whatever the
class name is) would implement it (and very few frames are scrolled) there won't
be much overhead. No more than we had before when using views

Comment 24

[Eric Vaughan]

Ok I will put the interface together, have you review it and then Chris can use
it to get his scrolled row groups working. Sound like a plan?

Comment 25

[leger]

Putting on PDT+ radar.

Comment 26

[Eric Vaughan]

Ok I have a fix for my part that is an interface that is implemented by scroll
frames. Once that is checked in I'll have to pass it off to Chris to make sure
he uses it. So I'm putting a date of next friday for him.

Comment 27

[Paul MacQuiddy]

spam: changing qa contact from ckritzer -> paulmac for xul bugs

Comment 28

[Eric Vaughan]

nsIScrollableFrame is now implemented and checed in. Search nsTableFrame for it
too see how it is used.

Comment 29

[Jim Roskind]

Is this still a blocker?  Is this still dogfood, or has enough of the problem
been resolved that either a) the whole bug is resolved; or b) the blocking and
dogfood stuff is resolved?

If this is no longer dogfood, please remove the word, and also remove the PDT
annotation.

Thanks,


Jim

Comment 30

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I don't know.  I haven't really been looking at panels since my internship
ended.  Eric (Krock)?

Comment 31

[karnaze (gone)]

I don't know if this is still dogfood or not but I do have changes locally that
work with the attachments and am waiting for an approval to checkin.

Comment 32

[chris hofmann]

kaarnaze checked in yesterday.
marking fixed to get on the testing radar.

lets try and get this checked out today.
thanks

Comment 33

[Paul MacQuiddy]

marking verified, all the test cases, attachments load without crashing using
12/14 builds

Comment 34

[leger]

BULK MOVE: Changing component from XUL to XP Toolkit/Widgets: XUL.  XUL 
component will be deleted.