149012 - Viewing page info, tab media crashes Mozilla [@ nsHTMLReflowState::CalculateHypotheticalBox][@ nsIFrame::GetStyleData]

Status: RESOLVED WORKSFORME

(Core :: XUL, defect, P1)

Description

[Michaël Hompus]

When viewing the page info -> media tab crashes Mozilla, tested with Mozilla 
RC3 on Windows XP Pro and Redhat Linux / KDE3

Comment 1

[Nagi Peters]

confirming on win98 with 2002060108
Talkback: TB6986051Y

Comment 2

[Ian Neal]

Confirming on Win2KSP2 using buildID 2002060208 causes crash (and Dr Watson),
talkback ID TB6986391Y

Comment 3

[Christopher Archer]

Confirming WinNT SP6a, build 2002052904, with Dr Watson.
Talkback ID TB6989298H

Comment 4

[Frederic Bezies]

Why not making it new, so ? :-)

With 3 talkbacks :-)

Comment 5

[beanladen]

Confirming RC3 on Linux 2.4.18

Comment 6

[Matthias Versen [:Matti]]

-> Layout ?

win2k debug 20020530.. :
nsIFrame::GetStyleData(nsStyleStructID eStyleStruct_Visibility, const 
nsStyleStruct * & 0x05876e58) line 577 + 3 bytes
nsHTMLReflowState::CalculateHypotheticalBox(nsIPresContext * 0x05874ff8, 
nsIFrame * 0x05872150, nsIFrame * 0x00000000, nsMargin & {...}, nsIFrame * 
0x058d75a0, nsHypotheticalBox & {...}) line 836
nsHTMLReflowState::InitAbsoluteConstraints(nsIPresContext * 0x05874ff8, const 
nsHTMLReflowState * 0x0012e610, int 10395, int 7635) line 1022
nsHTMLReflowState::InitConstraints(nsIPresContext * 0x05874ff8, int 10395, int 
7635, nsMargin * 0x00000000, nsMargin * 0x00000000) line 1971
nsHTMLReflowState::Init(nsIPresContext * 0x05874ff8, int -1, int -1, nsMargin * 
0x00000000, nsMargin * 0x00000000) line 326
nsHTMLReflowState::nsHTMLReflowState(nsIPresContext * 0x05874ff8, const 
nsHTMLReflowState & {...}, nsIFrame * 0x05872090, const nsSize & {...}, 
nsReflowReason eReflowReason_Resize, int 1) line 217
nsBoxToBlockAdaptor::Reflow(nsBoxLayoutState & {...}, nsIPresContext * 
0x05874ff8, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0, int 0, int 0, int 0, int 0, int 1) line 807
nsBoxToBlockAdaptor::RefreshSizeCache(nsBoxToBlockAdaptor * const 0x05a21ab4, 
nsBoxLayoutState & {...}) line 371 + 70 bytes
nsBoxToBlockAdaptor::GetAscent(nsBoxToBlockAdaptor * const 0x05a21ab4, 
nsBoxLayoutState & {...}, int & 0) line 580
nsSprocketLayout::GetAscent(nsSprocketLayout * const 0x02420e80, nsIBox * 
0x058d78fc, nsBoxLayoutState & {...}, int & 165) line 1520
nsContainerBox::GetAscent(nsContainerBox * const 0x058d78fc, nsBoxLayoutState & 
{...}, int & 165) line 589 + 38 bytes
nsBoxFrame::GetAscent(nsBoxFrame * const 0x058d78fc, nsBoxLayoutState & {...}, 
int & 0) line 1099 + 20 bytes
nsSprocketLayout::Layout(nsSprocketLayout * const 0x02420e80, nsIBox * 
0x058d78fc, nsBoxLayoutState & {...}) line 242
nsContainerBox::DoLayout(nsContainerBox * const 0x058d78fc, nsBoxLayoutState & 
{...}) line 605 + 34 bytes
nsBoxFrame::DoLayout(nsBoxFrame * const 0x058d78fc, nsBoxLayoutState & {...}) 
line 1208
nsBox::Layout(nsBox * const 0x058d78fc, nsBoxLayoutState & {...}) line 1052
nsStackLayout::Layout(nsStackLayout * const 0x02427448, nsIBox * 0x058d75d8, 
nsBoxLayoutState & {...}) line 331
nsContainerBox::DoLayout(nsContainerBox * const 0x058d75d8, nsBoxLayoutState & 
{...}) line 605 + 34 bytes
nsBoxFrame::DoLayout(nsBoxFrame * const 0x058d75d8, nsBoxLayoutState & {...}) 
line 1208
nsBox::Layout(nsBox * const 0x058d75d8, nsBoxLayoutState & {...}) line 1052
nsBoxFrame::Reflow(nsBoxFrame * const 0x058d75a0, nsIPresContext * 0x05874ff8, 
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) 
line 1000
nsRootBoxFrame::Reflow(nsRootBoxFrame * const 0x058d75a0, nsIPresContext * 
0x05874ff8, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0) line 242
nsContainerFrame::ReflowChild(nsIFrame * 0x058d75a0, nsIPresContext * 
0x05874ff8, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, 
int 0, unsigned int 0, unsigned int & 0) line 783 + 31 bytes
ViewportFrame::Reflow(ViewportFrame * const 0x058d7564, nsIPresContext * 
0x05874ff8, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0) line 577
IncrementalReflow::Dispatch(nsIPresContext * 0x05874ff8, nsHTMLReflowMetrics & 
{...}, const nsSize & {...}, nsIRenderingContext & {...}) line 942
PresShell::ProcessReflowCommands(int 0) line 6377
PresShell::FlushPendingNotifications(PresShell * const 0x05876120, int 0) line 
5184
nsEventStateManager::FlushPendingEvents(nsIPresContext * 0x05874ff8) line 4104
nsEventStateManager::GenerateDragGesture(nsIPresContext * 0x05874ff8, nsGUIEvent 
* 0x0012f8cc) line 1318
nsEventStateManager::PreHandleEvent(nsEventStateManager * const 0x058ef830, 
nsIPresContext * 0x05874ff8, nsEvent * 0x0012f8cc, nsIFrame * 0x059a9070, 
nsEventStatus * 0x0012f6d8, nsIView * 0x059b02b8) line 389
PresShell::HandleEventInternal(nsEvent * 0x0012f8cc, nsIView * 0x059b02b8, 
unsigned int 1, nsEventStatus * 0x0012f6d8) line 6115 + 43 bytes
PresShell::HandleEvent(PresShell * const 0x05876124, nsIView * 0x059b02b8, 
nsGUIEvent * 0x0012f8cc, nsEventStatus * 0x0012f6d8, int 0, int & 1) line 6044 + 
25 bytes
nsViewManager::HandleEvent(nsView * 0x059b02b8, nsGUIEvent * 0x0012f8cc, int 0) 
line 2076
nsView::HandleEvent(nsViewManager * 0x058757d8, nsGUIEvent * 0x0012f8cc, int 0) 
line 306
nsViewManager::DispatchEvent(nsViewManager * const 0x058757d8, nsGUIEvent * 
0x0012f8cc, nsEventStatus * 0x0012f7c8) line 1881 + 23 bytes
HandleEvent(nsGUIEvent * 0x0012f8cc) line 83
nsWindow::DispatchEvent(nsWindow * const 0x059b0354, nsGUIEvent * 0x0012f8cc, 
nsEventStatus & nsEventStatus_eIgnore) line 969 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f8cc) line 990
nsWindow::DispatchMouseEvent(unsigned int 300, unsigned int 0, nsPoint * 
0x00000000) line 4834 + 21 bytes
ChildWindow::DispatchMouseEvent(unsigned int 300, unsigned int 0, nsPoint * 
0x00000000) line 5091
nsWindow::ProcessMessage(unsigned int 512, unsigned int 0, long 3866808, long * 
0x0012fcec) line 3681 + 28 bytes
nsWindow::WindowProc(HWND__ * 0x000103d4, unsigned int 512, unsigned int 0, long 
3866808) line 1234 + 27 bytes
USER32! 77e01b60()
USER32! 77e01cca()
USER32! 77e083f1()
nsAppShellService::Run(nsAppShellService * const 0x010e2ef8) line 451
main1(int 2, char * * 0x002830b0, nsISupports * 0x00000000) line 1456 + 32 bytes
main(int 2, char * * 0x002830b0) line 1805 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e7d326()

Comment 7

[greer]

Attachment: Stacks from Talkback incidents reported earlier

Comment 8

[greer]

Reassinging to karnaze. Attinasi is not available. Chris, please correct me if I
am wrong and reassign per your whim. :-)

Comment 9

[greer]

Attachment: Values at nsHTMLReflowState::CalculateHypotheticalBox

FWIW - We are passing several NULL values at
nsHTMLReflowState::CalculateHypotheticalBox

For example:
	aBlockFrame = 0x00000000
	knowIntrinsicSize = 0 (0x00000000)

(Not sure how the code gets to nsIFrame::GetStyleData in Matti's stack.)

The stack says that this one is crashing at line 836 (may be off by one)

834	  if (knowBoxWidth) {
835	    aHypotheticalBox.mLeft = aHypotheticalBox.mRight - boxWidth;
836	    aHypotheticalBox.mLeftIsExact = PR_TRUE;

But it looks like .mLeftIsExact is still NULL following line 836:
	aHypotheticalBox = 0x0012f3c4
	   mLeft = 1625988707 (0x60ea9e63)
	   mRight = 8235360 (0x007da960)
	   mTop = 20 (0x00000014)
	   mLeftIsExact = . (0 0x00)
	   mRightIsExact = . (0 0x00)
	knowIntrinsicSize = 0 (0x00000000)

Comment 10

[greer]

Attachment: Stacks

Here is a copy of my stack (crashed from the ebay site with Trunk build
20020603xx) and Shill's crash (comment #3), from an earlier build.

Comment 11

[greer]

D'oh! Disregard the previous comment. It was intended for a different bug.

Comment 12

[greer]

To clarify the steps for this crash:
1) Go to the url listed ( http://www.hompus.nl/mozilla/bug6.html )
2) Select View | Page Info
3) Click on "Media" tab
4) crash

Adding testcase KW.

Comment 13

[joakim_46]

Confirming it with build 2002052306 under Windows ME. Talkback ID TB7004419K

Comment 14

[karnaze (gone)]

The crash occurs because an image is absolutely positioned inside a box and the 
nsHTMLReflowState code expects to find a containing block for image, but there 
isn't one. -->xul

Comment 15

[John Morrison]

So doing absolute positioning in XUL is something that You Just Don't Do (TM).
We shouldn't crash, but...

However, in this case, the element with absolute positioning is cloned into 
that XUL document from an external source. (That makes me nervous for other
reasons, and I've spoken with mstoltz, and he's going to look into that 
question).

Anyways, I have a bandaid fix for this particular crash scenario, and it's 
something that I think we should take on the branch for machv.

-> jaggernaut. Gimme some r=/sr= loving.

Index: browser/resources/content/pageInfo.js
===================================================================
RCS file: /cvsroot/mozilla/xpfe/browser/resources/content/pageInfo.js,v
retrieving revision 1.25.2.3
diff -u -r1.25.2.3 pageInfo.js
--- browser/resources/content/pageInfo.js       20 Apr 2002 14:48:47 -0000      
1.25.2.3
+++ browser/resources/content/pageInfo.js       6 Jun 2002 23:41:11 -0000
@@ -776,6 +776,7 @@
   if ("height" in item && item.height)
     newImage.height = item.height;
   newImage.removeAttribute("align"); // just in case.
+  newImage.removeAttribute("style"); // also, just in case [bug 149012]

   imageContainer.removeChild(oldImage);
   imageContainer.appendChild(newImage);

Comment 16

[John Morrison]

Attachment: patch; remove 'style' attribute from cloned node

Now removing 'style' may have other consequences that aren't preferable, but 
it's better than a crash. If there is a more complete fix, let's take that as
a separate issue.

Comment 17

[jag (Peter Annema)]

Why not turn this around and create a new <html:img> and then set the src
attribute? No worries about copying stuff we don't want. I take it we're already
setting height and width (either directly or through css) to make it fit within
the space we have for it.

Boris, Christian?

Comment 18

[Daniel Brooks [:db48x]]

we might as well at this point, yes. It's a seperate bug though, and one which 
I'll go ahead and fix. I'll probably lump it with the changes to one of the 
other bugs I'm working on, probably the mac menus bug. r=db48x for this bandaid 
though. It works around a crash so we should go ahead and get it in as soon as 
possible, and on the branch.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Either way makes sense to me.  I think the initial reason for cloning was to get
the right height and width, but we can just copy those over, I guess...

Comment 20

[jag (Peter Annema)]

I would advocate doing that (create new image, copy src, width and height) then,
instead of this patch that removes the style attribute. Does anyone have cycles
to do that? db48x, if you have time, please don't lump it in with that other
stuff, that would decrease the chance of this actually getting in.

Comment 21

[Daniel Brooks [:db48x]]

well, I'll see what I can do. I really just hate having multiple patches 
changing the same files. Maybe I should just lump those changes in with this 
fix. ;)

bz: you're right about the height/width bit. I think it was actually you who 
told me how to correctly clone the node, in order to fix that problem.

As a side note, should there be a fix so that the xul code won't crash in this 
situation?

Comment 22

[Daniel Brooks [:db48x]]

the changes to page info could be filed under bug 149777, leaving this bug to 
fix the underlying cause of the problem.

Comment 23

[Christian :Biesinger (don't email me, ping me on IRC)]

since that bug is inaccessible, it would be nice if you could do it in this bug 
(or a newly filed one...)

Comment 24

[Samir Gehani]

Nav triage team: nsbeta1+, adt2 rtm

Comment 25

[jag (Peter Annema)]

The fix for this bug is in bug 149777.

Comment 26

[jag (Peter Annema)]

Filed bug 153850 on the underlying issue. Marking this a dupe of bug 149777.

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

Comment 27

[Ian Neal]

Any reason why bug 149777 is not visible?

Comment 28

[timeless]

jag: you just duped into a security bug, that's not nice.

Comment 29

[jag (Peter Annema)]

Waaah!

WORKSFORME.