280009 - Crash when reloading hovering over animated gif and then reloading (using tr:hover,td:hover {display:table-caption}) [@ nsFrame::GetOffsetFromView] [@ nsIFrame::Invalidate] [@ nsImageFrame::FrameChanged]

Status: VERIFIED FIXED

(Core :: Layout: Tables, defect)

Description

[Martijn Wargers (dead)]

See upcoming testcase.

- hover over animated gif
- reload

Result-> crash

Reproduced with:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b) Gecko/20050126 Firefox/1.0+

Comment 1

[Martijn Wargers (dead)]

Attachment: Testcase

Comment 2

[Martijn Wargers (dead)]

Talkback ID: TB3320364X (the Talkback server is not ready yet at this moment)

Comment 3

[Martijn Wargers (dead)]

Ok, this is the talkback data:

nsFrame::GetOffsetFromView 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Clobber/mozilla/layout/generic/nsFrame.cpp,
line 2352]
nsImageFrame::FrameChanged 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Clobber/mozilla/layout/generic/nsImageFrame.cpp,
line 715]
nsImageListener::FrameChanged 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Clobber/mozilla/layout/generic/nsImageFrame.cpp,
line 2131]
imgRequest::FrameChanged 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Clobber/mozilla/modules/libpr0n/src/imgRequest.cpp,
line 370]
nsTimerImpl::Fire 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Clobber/mozilla/xpcom/threads/nsTimerImpl.cpp,
line 396]
nsAppStartup::Run 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Clobber/mozilla/toolkit/components/startup/src/nsAppStartup.cpp,
line 146]
main 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Clobber/mozilla/browser/app/nsBrowserApp.cpp,
line 60]
KERNEL32.DLL + 0x2893d (0x7c59893d)

Comment 4

[Hermann Schwab]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b) Gecko/20050127

Also crashs Mozilla Trunk, TB3330611Z, link below maybe valid in about 4 hours.

http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB3330611Z

Comment 5

[Hermann Schwab]

I just noticed that the <tr> is growing, while hovering, so the small gif moves
to the right. Don´t know if this is related to the crash, or to another bug.

Same seen on testcase of 
Bug  275917 td:hover {display:block} causes crash on reload when animated gif
inside [@ nsIFrame::Invalidate ]

https://bugzilla.mozilla.org/attachment.cgi?id=169515

Comment 6

[Mats Palmgren (inactive)]

Attachment: A bit of analysis (for first testcase)

Comment 7

[Mats Palmgren (inactive)]

Attachment: Testcase #2 (same as 1st but without :hover)

Crash occurs on Reload...

Comment 8

[Mats Palmgren (inactive)]

Attachment: Frame dump (for Testcase #2 - before Reload)

Comment 9

[Boris Zbarsky [:bzbarsky]]

OK, I'm lost in the little jungle of TableProcessChildren.  It's clear that
someone is screwing up the pseudo-frame state, but I can't tell who.

Simply not passing aTableCreator when we call ProcessChildren on the kids of the
caption helps some, but still not enough.

Comment 10

[OstGote!]

I crashed with Mozilla 1.8b build 2005012606 on WinNT4 at testcase 1 with
slightly different stack signatures, see TB reports
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB3326771X
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB3326777M
Change the summary to reflect that.

(In reply to comment #5)
> I just noticed that the <tr> is growing, while hovering, so the small gif moves
> to the right. Don´t know if this is related to the crash, or to another bug.

For me the images moves DOWN if I hover it. I'm curious, can someone explain me
my the image moves?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Note that the actual stack signatures are irrelevant -- they all point to a
deleted image frame being accessed.

The image moves because we screw up removing table pseudo-frames; we have
existing bugs on that.

Comment 12

[Bernd]

Is this a regression or an old crash, like it also crashes FF 1.0?

Comment 13

[Hermann Schwab]

(In reply to comment #12)
> Is this a regression or an old crash, like it also crashes FF 1.0?

Firefox 1.0 on Win98
1st testcase: load, hover, reload
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB3387549K
2nd testcase: load, reload
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=TB3387621E

Comment 14

[Bernd]

bernd: the only real way we can mess this up is to create an image frame and
then lose it from the frame tree altogether...

Comment 15

[Bernd]

Attachment: testcase

It looks like we create the inner caption but don't append it to the frame tree

Comment 16

[Bernd]

Attachment: patch

Comment 17

[Boris Zbarsky [:bzbarsky]]

Bernd, why does that help in this case?  What's aParentFrame in this case?

Comment 18

[Bernd]

First note: this function is only called for captions. Its probably somehow
misnamed but I dont have good idea how to name it.

what does this code

-  if (nsLayoutAtoms::tableOuterFrame == aParentFrame->GetType()) {
-    return aParentFrame;
-  }
-  return aParentFrame->GetParent();

if the parent is the outer table frame use it as caption parent, for *all* other
use the  grandparent. This works usually fine as the caption in content space is
a table child so we end here usually with a table frame take the parent of the
table which is the outer and every thing is in the green region. But breaks if
some strange table frame gets display: table-caption.

What will the code below do?

+  if (nsLayoutAtoms::tableFrame == aParentFrame->GetType()) {
+    return aParentFrame->GetParent();;
+   }
+  return aParentFrame;

If the caption is below a table frame take the grandparent and attach the
caption to the outer table frame. For all other cases take what you get as a
parent build then the necessary pseudo frames.

why do we crash, so we start with a caption, then we create below it another
caption and take as a parent not the caption but the parent of the caption so we
have two captions below the same parent...
So instead of 

P       we have P
Tbl C1          Tbl C1 C2
    C2

and we don't process then correctly the second caption so it gets lossed from
the frame tree.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 176919 [details] [diff] [review]
patch

How about renaming the method to AdjustCaptionParentFrame?

Comment 20

[Bernd]

fix checked in the checkin comment wrongly references bug 284575

Comment 21

[timeless]

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