If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Crash [@ ProcessPseudoFrame 6cf4920e] on changing display type to table-row on hidden inputs

VERIFIED FIXED

Status

()

Core
Layout: Tables
--
critical
VERIFIED FIXED
13 years ago
6 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: Bernd)

Tracking

({crash, testcase})

Trunk
x86
Windows XP
crash, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(4 attachments)

(Reporter)

Description

13 years ago
See upcoming testcase.
When you hover over the text in the testcase twice, it makes Mozilla crash.

Talkback ID: TB3349432Z

ProcessPseudoFrame 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Clobber/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 2287]
ProcessPseudoFrames 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Clobber/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 2443]
nsCSSFrameConstructor::ContentInserted 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Clobber/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 9404]
nsCSSFrameConstructor::RecreateFramesForContent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Clobber/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 11793]
nsCSSFrameConstructor::ProcessRestyledFrames 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Clobber/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 10315]
nsCSSFrameConstructor::RestyleElement 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Clobber/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 10382]
nsCSSFrameConstructor::ProcessPendingRestyles 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Clobber/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 13795]
nsCSSFrameConstructor::RestyleEvent::HandleEvent 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.0_Clobber/mozilla/layout/base/nsCSSFrameConstructor.cpp,
line 13859]
SHELL32.dll + 0x520c24 (0x778b0c24)
(Reporter)

Comment 1

13 years ago
Created attachment 172700 [details]
Testcase
(Reporter)

Updated

13 years ago
Severity: normal → critical
I assume the nested inputs are needed, right?

Also, is this a recent regression (from the fix for bug 269566?).
(Reporter)

Comment 3

13 years ago
(In reply to comment #2)
> I assume the nested inputs are needed, right?
Yes, otherwise it won't crash.

> Also, is this a recent regression (from the fix for bug 269566?).
Yes, seems like it. It doesn't crash in a 2005-01-24 build, but it does crash in
a 2005-01-25 build.

I really don't know what's going on here....  We're somehow managing to end up
with a null parent but non-null child lists in the pseudo-frames.

I had thought the patch for bug 280708 might help here, but it does not.
(Assignee)

Comment 5

13 years ago
the frist hover produces 

frame: TableRow(input)(2) (0356C1E0) style: 035901EC {}
Wrong parent style context:  style: 03590C80 {}
should be using:  style: 0356C0F4 :-moz-table-row-group {}
(Assignee)

Comment 6

13 years ago
<style>
body:hover input{display:table-row;}
</style>
</head>
<body>
Hovering over this text should not crash Mozilla
<input type="hidden"><input type="hidden">
</body></html>

the first question that arises is why we end in table pseudoframe creation at all?
Input tags should create input frames based on the tag and not on the display type.
<input type="hidden"> is however special, it does not create a frame, so during
 frame creation we pass the first if but return a NS_OK and no frame so we step
through all other frames and end in the display type section.
 
    else if (nsHTMLAtoms::input == aTag) {
    if (!aHasPseudoParent && !aState.mPseudoFrames.IsEmpty()) {
      ProcessPseudoFrames(aPresContext, aState.mPseudoFrames, aFrameItems); 
    }
    isReplaced = PR_TRUE;
    rv = CreateInputFrame(aPresShell, aPresContext,
                          aContent, newFrame, aStyleContext);
  }
where we finally create a row and have a completely horked PseudoFrame creation.
(Assignee)

Comment 7

13 years ago
we create a table cell pseudo after
http://lxr.mozilla.org/seamonkey/source/layout/base/nsCSSFrameConstructor.cpp#3002
which horks completely the pseudo stack.
(Assignee)

Comment 8

13 years ago
Created attachment 174874 [details] [diff] [review]
debug code that I used
(Assignee)

Comment 9

13 years ago
Created attachment 174877 [details] [diff] [review]
patch
Assignee: nobody → bernd_mozilla
Status: NEW → ASSIGNED
Attachment #174877 - Flags: superreview?(bzbarsky)
Attachment #174877 - Flags: review?(bzbarsky)
Comment on attachment 174877 [details] [diff] [review]
patch

I'd prefer that the QI only happen when the tag is nsHTMLAtoms::input.	With
that, and a comment explaining that <input type="hidden"> is the only time
<input> doesn't get a special frame, r+sr=bzbarsky.
Attachment #174877 - Flags: superreview?(bzbarsky)
Attachment #174877 - Flags: superreview+
Attachment #174877 - Flags: review?(bzbarsky)
Attachment #174877 - Flags: review+
Also note that XBL form controls are effectively defunct for now... Add comments
about them as needed to the code that instantiates them, but no need to worry
about them past that.
(Assignee)

Comment 12

13 years ago
Created attachment 174961 [details] [diff] [review]
revised patch for checkin
(Assignee)

Comment 13

13 years ago
Martijn: There are other special frames that could in principle create a
similiar problem, they are listed below. If you can still create crash testcases
then we need also to plug the remaining hooles.

    tag == nsHTMLAtoms::img ||
    tag == nsHTMLAtoms::br ||
    tag == nsHTMLAtoms::wbr ||
    tag == nsHTMLAtoms::input ||
    tag == nsHTMLAtoms::textarea ||
    tag == nsHTMLAtoms::select ||
    tag == nsHTMLAtoms::object ||
    tag == nsHTMLAtoms::applet ||
    tag == nsHTMLAtoms::embed ||
    tag == nsHTMLAtoms::fieldset ||
    tag == nsHTMLAtoms::legend ||
    tag == nsHTMLAtoms::frameset ||
    tag == nsHTMLAtoms::iframe ||
    tag == nsHTMLAtoms::noframes ||
    tag == nsHTMLAtoms::spacer ||
    tag == nsHTMLAtoms::button ||
    tag == nsHTMLAtoms::isindex;    
(Assignee)

Comment 14

13 years ago
The hot candidates are inside ConstructHTMLFrame, when we don't construct a
frame for a tag like iframes when frames are not allowed.
Note that iframes when frames are not allowed are known-broken... we have bugs
on that already.
(Assignee)

Comment 16

13 years ago
the frame behaviour is controlled by the browser.frames.enabled preference.
(Assignee)

Comment 17

13 years ago
Ehmm, there is a little problem with that preference bug 107911 that would
forbid that you will hit the crash as your browser will not start anyway.
(Reporter)

Comment 18

13 years ago
With a quick test:
http://nova.serverway.net/~mw22/test/mozilla/280217_crash/
it seems to crash also with the <noframes> tag, but not with the other tags.
(Assignee)

Comment 19

13 years ago
Fix checked in. The new testcase is now referenced in bug 240129 as it will be
fixed by the patch there.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Verified FIXED with the testcase
https://bugzilla.mozilla.org/attachment.cgi?id=172700 using Mozilla/5.0
(Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050304
Status: RESOLVED → VERIFIED
(Assignee)

Comment 21

13 years ago
Comment on attachment 174874 [details] [diff] [review]
debug code that I used

Boris, I need that more frequently than I wished.
Attachment #174874 - Flags: superreview?(bzbarsky)
Attachment #174874 - Flags: review?(bzbarsky)
Comment on attachment 174874 [details] [diff] [review]
debug code that I used

>Index: nsCSSFrameConstructor.cpp
>+#ifdef DEBUG
>+  if(gTablePseudoFrame) {
>+    printf("*** ProcessPseudoFrames complete leave:%p***\n",highestFrame);

Want to indicate in the string that %p is the highestFrame?

>+    else if (nsLayoutAtoms::tableRowFrame == aHighestType) 
>+      printf("Row");
>+    else if (IS_TABLE_CELL(aHighestType)) 
>+      printf("Row");

"Cell" in that last else if, right?

r+sr=bzbarsky with that.
Attachment #174874 - Flags: superreview?(bzbarsky)
Attachment #174874 - Flags: superreview+
Attachment #174874 - Flags: review?(bzbarsky)
Attachment #174874 - Flags: review+

Comment 23

13 years ago
builds from Feb. 23rd onwards occationally crash in nsPluginInstanceOwner::Destroy
TB4316693K
Looking at the checkins between 2005-02-22 04:00:00 and 2005-02-23 04:00:00,
this checkin looks like it may be involved. Please take a look at the stack.
There have been 20 new crashes reported with that stack with builds (seamonkey
and ff, trunk)
Crash Signature: [@ ProcessPseudoFrame 6cf4920e]
You need to log in before you can comment on or make changes to this bug.