310662 - Evil testcase causing crash in [@ nsCSSFrameConstructor::ConstructTableRowGroupFrame]

Status: VERIFIED FIXED

(Core :: Layout: Tables, defect, P1)

Description

[Martijn Wargers (dead)]

See upcoming testcase.
Hovering over the text causes a crash for me in current trunk build.

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Comment 2

[Martijn Wargers (dead)]

From talkback ID: TB9941995M

nsCSSFrameConstructor::ConstructTableRowGroupFrame 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 3591]
ProcessPseudoFrames 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 2717]
FindPreviousAnonymousSibling 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 8086]
nsCSSFrameConstructor::ProcessChildren 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 11638]
nsCSSFrameConstructor::AttributeChanged 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 10518]
nsCSSFrameConstructor::AttributeChanged 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 10552]

Comment 3

[Martijn Wargers (dead)]

This regressed between 2005-09-18 and 2005-09-20:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-09-18+07%3A00%3A00&maxdate=2005-09-20+07%3A00%3A00&cvsroot=%2Fcvsroot
Bug 11011 seems to me the most likely candidate.

Comment 4

[Bernd]

Martijn suspects bug 11011

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

We're now producing useful non-html frames here, so we should really create
pseudo-frames for them.  And since all this stuff is in content, we can!

Comment 6

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

Comment on attachment 198246 [details] [diff] [review]
Fix

>-  nsresult rv = AdjustParentFrame(aContent, display, adjParentFrame,
>+  nsresult rv = AdjustParentFrame(aContent, aStyleContext, adjParentFrame,

Shouldn't you be using styleContext instead of aStyleContext?  That's what
corresponds to display, anyway.

Comment 7

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

Also, you may as well end the input section with a |return PR_TRUE;| and remove
it from the later list.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Er, yes.  That should use |display|.

Comment 9

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

sr=dbaron with those changes.  (Also, the "in sync" comments might be better off
being inside CreateInputFrame and CreateHTMLImageFrame, or in both places.)

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated to comments

Comment 11

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.

Comment 12

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using build 2005-10-05-05 SeaMonkey trunk, Windows XP on the
testcase at https://bugzilla.mozilla.org/attachment.cgi?id=198114

Comment 13

[Kurt Weinschenker]

This bug apparently *is not* fixed for Camino users.

The evil testcase crashes a recent Camino nightly trunk build from 20051002. 
Haven't tried it with other versions -- yet.  Will test it though, over the next
week or so.  Mac OS 10.3.9.

Request this one be left open for Camino users to note.  Have a nice day.

Comment 14

[Boris Zbarsky [:bzbarsky]]

> The evil testcase crashes a recent Camino nightly trunk build from 20051002. 

Kurt, I checked in the fix at 2005-10-04 16:56 PDT or so.  See comment 11.

Now you're complaining that the bug is not fixed in a Camino build from
2005-10-02.  Of course it's not fixed there!  It didn't get fixed until two days
after that.

You have a nice day too.

Comment 15

[Kurt Weinschenker]

WFM with Camino 2005100504 (v1.0a1+).  You're right Boris, sorry for the spam, all.

Comment 16

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

Comment on attachment 198525 [details] [diff] [review]
Updated to comments

belated r=biesi :)

though I spotted an else-after-return here:

@@ -3316,35 +3319,52 @@ nsCSSFrameConstructor::GetParentFrame(ns
+IsSpecialHTMLContent(nsIContent* aContent, nsStyleContext* aStyleContext)
+      if (NS_FORM_INPUT_HIDDEN == type) {
+        return PR_FALSE; // input hidden does not create a special frame
+      }
+      else if (NS_FORM_INPUT_IMAGE == type) {

Comment 17

[Bernd]

I touched the code after this, I will apply the necessary changes to fit the review comment.