30152 - choosing columns to show and hide in manage bookmarks crashes

Status: VERIFIED DUPLICATE of bug 33524

(Core :: XUL, defect, P1)

Description

[Claudius Gayle]

Overview Description: 

  There's something real evil about the little arrow to the right of the column headers in the manage bookmarks window.

Steps to Reproduce: 

1) Open Manage bookmarks. Click the little triangle to the far right of the column headers to drop down a list of the columns.

2) Select (uncheck) a column.

3) Wait patiently -or- click on that arrow again.


Actual Results: 

	Upon selecting an item and dismissing that menu, UI becomes sluggish and unresponsive. It you wait patiently you will 
crash. If you become impatient you can simply click the arrow again to crash immediately

Expected Results: 

	The window should redraw with the column of your choice missing or added as appropriate.

Build Date & Platform Bug Found: 

	All platforms with 2000030208 builds.

Comment 1

[Robert John Churchill]

David?

Comment 2

[David Hyatt]

You've miscopied the code from mailnews.  Probably shouldn't crash, but if you 
patch your XUL, it should work.

Comment 3

[Robert John Churchill]

I don't know anything about this... I didn't add this in the first place. Slamm?

Comment 4

[Claudius Gayle]

*** Bug 30847 has been marked as a duplicate of this bug. ***

Comment 5

[aaronr]

I did some debugging on this.  Looks like the trap is because of a mismatch 
between count variables.  On debug it hits an assert.  Here is the call stack:
NTDLL! 77f76274()
nsDebug::Assertion() line 189 + 13 bytes
FixedTableLayoutStrategy::AssignPreliminaryColumnWidths() line 92 + 32 bytes
BasicTableLayoutStrategy::Initialize() line 101
nsTableFrame::Reflow() line 1552
nsTreeFrame::Reflow() line 410 + 25 bytes
nsContainerFrame::ReflowChild() line 638 + 31 bytes
nsTableOuterFrame::IR_InnerTableReflow() line 703 + 40 bytes
nsTableOuterFrame::IR_TargetIsInnerTableFrame() line 476 + 31 bytes
nsTableOuterFrame::IR_TargetIsChild() line 443 + 31 bytes
nsTableOuterFrame::IncrementalReflow() line 423 + 35 bytes
nsTableOuterFrame::Reflow() line 936 + 31 bytes
nsTreeOuterFrame::Reflow() line 136 + 25 bytes
nsBoxFrameInner::FlowChildAt() line 2107
nsBoxFrameInner::FlowChildren() line 1372
nsBoxFrame::Reflow() line 1099
nsContainerFrame::ReflowChild() line 638 + 31 bytes
RootFrame::Reflow() line 331
nsContainerFrame::ReflowChild() line 638 + 31 bytes
ViewportFrame::Reflow() line 531
nsHTMLReflowCommand::Dispatch() line 145
PresShell::ProcessReflowCommands() line 2031
ReflowEvent::HandleEvent() line 1931
HandlePLEvent() line 1943
PL_HandleEvent() line 526 + 10 bytes
PL_ProcessPendingEvents() line 487 + 9 bytes
_md_EventReceiverProc() line 975 + 9 bytes
USER32! 77e71268()
01028be0()

You'll notice in AssignPreliminaryColumnWidths, that it is calling 
nsTableFrame::GetColCount() to get the number of columns.  This will, in turn 
call cellMap->GetColCount.  This will always come back as 8.  The assert is 
happening in the for loop because GetColFrame() is returning null and at that 
time, the value of colX is 7.  But the problem is that inside GetColFrame it is 
getting the column frame from mColFrames, and not the cellMap.  The count for 
mColFrames.mCount is only 7 at this point.  Thus, if it were using the 
mColFrames counter, it wouldn't have hit this point.  When I restarted, though, 
I could see that mColFrames.mCount = cellMap.mCount at 8.  So I put a data write 
break point in to see who changed it to 7.  That happens in this call chain:

nsVoidArray::RemoveElementAt() line 221
nsTableFrame::RemoveCol() line 676
nsTableColGroupFrame::RemoveFrame() line 419
FrameManager::RemoveFrame() line 632
nsCSSFrameConstructor::ContentRemoved() line 7124 + 63 bytes
nsCSSFrameConstructor::RecreateFramesForContent() line 8687 + 28 bytes
  (calls the RecreateFramesForContent that is because restyle == true, the
   maxHint is NS_STYLE_HINT_FRAMECHANGE)
nsCSSFrameConstructor::AttributeChanged() line 7753 + 16 bytes
StyleSetImpl::AttributeChanged() line 1014
PresShell::AttributeChanged() line 2569 + 57 bytes
nsXULDocument::AttributeChanged() line 1449
nsXULElement::SetAttribute() line 2532
nsXULElement::SetAttribute() line 1234 + 35 bytes
ElementSetAttribute() line 258 + 26 bytes
js_Invoke() line 665 + 26 bytes
js_Interpret() line 2292 + 15 bytes
js_Invoke() line 681 + 13 bytes
js_Interpret() line 2292 + 15 bytes
js_Invoke() line 681 + 13 bytes
js_InternalInvoke() line 754 + 19 bytes
JS_CallFunctionValue() line 2787 + 31 bytes
nsJSContext::CallEventHandler() line 562 + 33 bytes
nsJSEventListener::HandleEvent() line 128 + 57 bytes
nsEventListenerManager::HandleEventSubType() line 697 + 19 bytes
nsEventListenerManager::HandleEvent() line 1456 + 35 bytes
nsXULElement::HandleDOMEvent() line 2987
nsMenuFrame::Execute() line 1125
nsMenuFrame::HandleEvent() line 285
PresShell::HandleEvent() line 2950 + 38 bytes
nsView::HandleEvent() line 799
nsView::HandleEvent() line 784
nsViewManager2::DispatchEvent() line 1216
HandleEvent() line 69
nsWindow::DispatchEvent() line 493 + 10 bytes
nsWindow::DispatchWindowEvent() line 514
nsWindow::DispatchMouseEvent() line 2938 + 21 bytes
ChildWindow::DispatchMouseEvent() line 3156
nsWindow::ProcessMessage() line 2243 + 24 bytes
nsWindow::WindowProc() line 671 + 27 bytes
USER32! 77e71268()
JS3250! 0031004b()

The attribute that it is setting is "hidden" and the value is "true".  You'll 
see a note in this call stack why a call was made to 
nsCSSFrameConstructor::RecreateFramesForContent().  This call is NEVER made for 
mail/news, that is why they aren't encountering this problem.  They aren't 
having a colframe removed like the bookmarks window is.

--Aaron
[e-mail]

Comment 6

[aaronr]

Looking at nsCSSFrameConstructor::AttributeChanged to see the differences 
between messenger and bookmarks behavior, I noted the value of the aAttribute 
parameters.  This is the sequence of attributes entering this function during 
the mouseclick on the menu to remove a column:
*checked
*menuactivate
*menutobedisplayed
*hidden (value of ahint at line 7705 is 0x04, maxint at line 7747 is 0x04)
*open
*menuactivate

For bookmarks, on the mouse click, I got this sequence:
*checked
*menuactivate
*menutobedisplayed
*hidden (ahint at line 7705 is 0x02, maxint at line 7747 is 0x02)
hidden (ahint at line 7705 is 0x04, but then maxint at line 7747 is 0x05), which 
will cause the column to be removed and the colcount to go down to 7, 
contributing to the trap
*open
*menuactivate

This doesn't mean much to me, but it might to someone else.  We definitely 
aren't getting the same kind of return 
from styledContent->GetMappedAttributeImpact as messenger.  The whole "hidden" 
attribute is handled differently.  I don't know why this would be different than 
messenger.  I even tried putting the bookmarks tree inside 2 boxes like 
messenger has.  Made no difference.

--Aaron

Comment 7

[aaronr]

Problem is in the bookmarks.css file.  Debugged it the style being applied to 
the attribute.  Then looked in the CSS to see what the style was.  When I 
comment out this rule, things work just dandy.

treecol[hidden="true"]
{
  display: none;
}

Messenger chrome doesn't have this rule.  Must be handled internally by the tree 
code.

--Aaron

Comment 8

[don]

Steve, is this REALLY our bug and not a tree problem?  Hmmmm ... well, whatever
it is, let's try to fix it for M15.  Move it out to M16 if you're swamped.

Comment 9

[Robert John Churchill]

I've removed the CSS from "bookmarks.css" that [e-mail] pointed out as 
causing the crash.

Might want to reassign this bug to someone more familiar with tables/CSS so that 
they can fix the real problem.

Comment 10

[Matthew T (active 1999-2002)]

This is happening to me in the Bookmarks *and* the Messenger three-pane window. 
Build 2000033114, MacOS 8.6. (I also get crashes when closing the History and 
Address Book windows, after clicking on the column headers in those, even though 
they don't have hide/show widgets right now; that might be this bug, or it might 
be a separate one.)

Moving to `XP Toolkit/Widgets: Trees' category, since it's not just Bookmarks, 
and since I nearly reported a dup of this bug when I didn't find it there.

Comment 11

[Matthew T (active 1999-2002)]

This is happening to me in the Bookmarks *and* the Messenger three-pane window. 

Build 2000033114, MacOS 8.6. (I also get crashes when closing the History and 

Address Book windows, after clicking on the column headers in those, even though 

they don't have hide/show widgets right now; that might be this bug, or it might 

be a separate one.)



Moving to `XP Toolkit/Widgets: Trees' category, since it's not just Bookmarks, 

and since I nearly reported a dup of this bug when I didn't find it there.

Comment 12

[Claudius Gayle]

*** Bug 34690 has been marked as a duplicate of this bug. ***

Comment 13

[don]

Re-assinging to component owner.  This isn't slamm's bug.

Comment 14

[David Hyatt]

*** This bug has been marked as a duplicate of 33524 ***

Comment 15

[Claudius Gayle]

grrr, VERIFIED Dupe.