317480 - Mail Compose Window is Horked [nsGrid::GetFirstAndLastRow]

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Scott MacGregor]

This was caused by Bug #306911. The grid in the mail compose window is no longe rendering correctly. 

Screen shot coming up.

Comment 1

[Scott MacGregor]

Attachment: screen shot of the regression

Comment 2

[Scott MacGregor]

In addition to the UI being horked, I'm also now crashing as well when replying to messages.

nsGrid::GetFirstAndLastRow(nsBoxLayoutState & {...}, int & 0, int & -1, nsGridRow * & 0x06073b7c, nsGridRow * & 0x00000000, int 1) line 708 + 12 bytes
nsGridRowLeafLayout::PopulateBoxSizes(nsIFrame * 0x04d63584, nsBoxLayoutState & {...}, nsBoxSize * & 0x00000000, nsComputedBoxSize * & 0x00000000, int & 0, int & 0, int & 0) line 237
nsSprocketLayout::Layout(nsSprocketLayout * const 0x049305b0, nsIFrame * 0x04d63584, nsBoxLayoutState & {...}) line 265
nsGridRowLeafLayout::Layout(nsGridRowLeafLayout * const 0x049305b0, nsIFrame * 0x04d63584, nsBoxLayoutState & {...}) line 363
nsBoxFrame::DoLayout(nsBoxFrame * const 0x04d63584, nsBoxLayoutState & {...}) line 1089 + 34 bytes
nsIFrame::Layout(nsBoxLayoutState & {...}) line 802
nsSprocketLayout::Layout(nsSprocketLayout * const 0x035ae1c8, nsIFrame * 0x046bcf10, nsBoxLayoutState & {...}) line 547
nsBoxFrame::DoLayout(nsBoxFrame * const 0x046bcf10, nsBoxLayoutState & {...}) line 1089 + 34 bytes
nsIFrame::Layout(nsBoxLayoutState & {...}) line 802
nsStackLayout::Layout(nsStackLayout * const 0x04d56378, nsIFrame * 0x042ed95c, nsBoxLayoutState & {...}) line 321
nsGridLayout2::Layout(nsGridLayout2 * const 0x04d56378, nsIFrame * 0x042ed95c, nsBoxLayoutState & {...}) line 78 + 17 bytes
nsBoxFrame::DoLayout(nsBoxFrame * const 0x042ed95c, nsBoxLayoutState & {...}) line 1089 + 34 bytes
nsIFrame::Layout(nsBoxLayoutState & {...}) line 802
nsSprocketLayout::Layout(nsSprocketLayout * const 0x02b35aa8, nsIFrame * 0x042f5274, nsBoxLayoutState & {...}) line 547
nsBoxFrame::DoLayout(nsBoxFrame * const 0x042f5274, nsBoxLayoutState & {...}) line 1089 + 34 bytes

Comment 3

[Scott MacGregor]

I'm now unable to reply to messages which is a smoketest blocker so I'm adding the right keyword and severity foo to show up as a smoketest blocker.

Comment 4

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

Attachment: possible patch

I asked mscott to try this patch (he's building now), since based on the stack it looks like there's a <row> or <column> inside a <grid> without an intervening <rows> or <columns>.  And now that I think about it, that probably used to work and it probably shouldn't be broken.

Comment 5

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

(Well, I'm not entirely sure that the code makes sense that way, but the change wouldn't be needed to fix what I was fixing, at least.)

Comment 6

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

Turns out that doesn't help.

Comment 7

[Boris Zbarsky [:bzbarsky]]

I have to admit to being a little confused -- I don't see an obvious <grid> in that part of the mailnews window...  mscott, do you happen to know what grid is involved?

Comment 8

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

It's the <listbox> and children, which are grids via display type and XBL.

Comment 9

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

And the problem is that <listrows> contains <listboxbody>, which is the normal thing the listbox XBL binding does.  So for now we need to let grid groups contain grid groups, although I'm a little scared of what happens if the two have different -moz-box-orient.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Ah.  So the issue there is that the DOM looks like (including XBL anon content):

  listbox
    xul:listcols
    listcols
    xul:listrows
      xul:listboxbody
        listitem
        listitem
        ...

Both the xul:listrows and the xul:listboxbody are display:-moz-grid-rowgroup, so the listboxbody is not treated as part of the grid with this patch.

Realistically, I believe it _might_ make sense to allow rowgroups to contain other rowgroups...  At least I don't see an a priori reason to disallow that, and now that I think about it some of the grid code may even deal with this sanely (eg count rows in nested rowgroups when counting for a rowgroup).

Comment 11

[Boris Zbarsky [:bzbarsky]]

Mid-air collision there... ;)

Yeah, the box-orient thing is not so happy... Would exposing that on the grid layout be doable?

Comment 12

[Scott MacGregor]

Attachment: the fix

David came up with this patch which fixes the problem.

Comment 13

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

I checked in the above patch so that things work again.

Comment 14

[[e-mail]]

I thought nested row groups are used somewhere else but I forget where.

Comment 15

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

I filed bug 323103 on further investigation of comment 9.