57026 - Crash on a form field when position:relative

Status: VERIFIED FIXED

(Core :: XUL, defect, P1)

Description

[Ilya Konstantinov]

A <form>'s <input> field in <span style="position:relative"> crashes Mozilla.

<span style="position:relative">
<form action="test.html" method="post">
<input type="text" name="fullname" value="">
</form>
</span>

Comment 1

[Doron Rosenberg (IBM)]

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

Comment 2

[Doron Rosenberg (IBM)]

--> layout

on win32 200101704, it casues wierd behaviour and crashes on reload (marking os all)

Comment 3

[ekrock's old account (dead)]

Reassigning to pierre for initial evaluation to get off clayton's list.

Comment 4

[Ilya Konstantinov]

To clarify my report, the crash isn't upon entering the page but upon leaving it
and going to another URL. (sorry for the dupe)

Comment 5

[Pierre Saslawsky]

The crash happens because nsBoxFrame::Destroy() calls nsContainerFrame::Destroy() 
which tries to delete a view which has already been deleted. Note that an 
assertion fires off 4 times before the page is displayed:
  ###!!! ASSERTION: translation failed: 'TranslatePointTo(origin, containingView,
  parentView)', file nsContainerFrame.cpp, line 385

If you change the position:relative to position:static, there is no assertion and 
no crash later on.

I'm not familiar with the view and frame hierarchy in the text edit field so I'm 
going to reassign to XP Toolit/Widgets.  CCd evaughan who last worked in 
nsContainerFrame::PositionFrameView(): a comment mentions a total hack to fix 
another hack.  CCd rods too because it's his area of expertise too.

Comment 6

[Pierre Saslawsky]

Attachment: stack trace

Comment 7

[rods (gone)]

This looks like it should go to evaughan

Comment 8

[Peter Trudelle]

So I guess we should assign it to him.  How common is this on top100 sites?
Does it occur in opt N6 verification builds?

Comment 9

[Eric Vaughan]

This does not crash for me but it does assert. It asserts on line 385. The
problem is the line:

NS_VERIFY(TranslatePointTo(origin, containingView, parentView), "translation
failed");

"containingView" can be equal to or the parent of "parentView" but the arguments
to TranslatePointTo are "origin", "childView", "parentView". It looks like the
containingView and parentView are being passes in reverse order. But I don't
understand what this code is supposed to do. Rod this is for drop downs. What is
the code for?

Comment 10

[Pierre Saslawsky]

Maybe you missed the comment from [2000-10-17 23:40]...
To reproduce:
- load any page
- go to the URL above
- click the Back button
==> crash

Comment 11

[John Morrison]

Yes, it crashes for me when I _leave_ the page. (Some variations on the test 
case do not crash though.)

If I apply left/top properties, the actual text is oddly displaced twice
from the visible outline of the input box (i.e., if top:200px, the outline is
at 200px, but text entered in the control is displayed at 400px). I'll attach 
a testcase.

I don't believe this pattern is common on top100 sites, which more commonly use 
position:absolute to place DIVs that contain a collection of text, images and 
form controls (e.g. ESPN sites). However, this bug makes me a little uneasy.

Comment 12

[John Morrison]

Attachment: testcase using a DIV block, and top:200px, left:200px

Comment 13

[Peter Trudelle]

Only a little uneasy?  Not sure that meets our current criteria. ;-) Is it in
use at all on top100 sites?  Sounds like it wouldn't have to be commonly used to
be a common crash.  Does this show up on Talkback with any frequency?  Is anyone
seeing this in their normal use?

Comment 14

[Ilya Konstantinov]

First of all, it creates a problem on my company's site :)
And besides, that pretty much says position:relative crashes Mozilla. Will we
have Mozilla in those "Bugs in browsers" pages which'll suggest not to use
"position:relative" in Mozilla cause it's flaky? I hoped we'll have a sane
browser to develop for ...

Comment 15

[Peter Trudelle]

cc'ing ekrock for a khaki opinion. How critical is position:relative?

Comment 16

[ekrock's old account (dead)]

position:relative is critical. It is one of the two most fundamental ways 
content is explicitly positioned in CSS (by position:relative and 
position:absolute) as opposed to implicitly positioned through the box model. It 
was part of the CSSP draft released in early '97 and has been supported by 
browsers since Nav4.x and IE4.x. We definitely can't pull position:relative for 
RTM, so as a crasher that potentially interferes with the adoption of a key 
feature of a standard, this should be a high-priority candidate for a fix, even 
if the specific example in which it occurs seems at first blush to be an edge 
case in some way. (We'd really hate to take the risk and turn out to be 
wrong.) Crashers on the use of CSS markup in Nav4 were a key factor in 
delaying the adoption of CSS on the web and something we don't want to 
repeat. Crashers are still valid candidates for fixing this week! Let's please 
fix this if at *all* possible. evaughn--if you don't have the cycles, let's find 
another person.

Comment 17

[Peter Trudelle]

rtm need info, p1 for m18

Comment 18

[Andreas Franke (gone)]

Attachment: self-destroying testcase: crashes without user interaction

Comment 19

[rods (gone)]

I commented out the two asserts and set the coords manually:
//NS_VERIFY(TranslatePointTo(origin, containingView, parentView), "translation 
failed");
origin.x = origin.y = 150;

and it doesn't change anything. It still crashes. I think the problem is how the 
views are created for GfxTextControls.

I put a debug statement in nsContainerFrame::Destroy that prints out the view it 
retrieves and then a print statement in the destructor of the nsView. You can 
see from theoutput below that container 00E7F124 gets view 02AE87D0 which has 
already been destroyed. Why? I am not sure. But there MUST be something wacky 
about the frame and view hierarchies for the GfxTextControl.

****** Container 00E6A520 has view 02A8DDF0
****** Container 00E6A594 has view 00000000
****** Container 00E6A63C has view 02AAF530
****** Container 00E6A55C has view 02AB1030
****** Container 00E6B4A0 has view 02AC10C0
********* Deleting nsView::~nsView 02AC10C0
********* Deleting nsView::~nsView 02AE87D0
********* Deleting nsView::~nsView 02AE9800
****** Container 00E7EF60 has view 00000000
****** Container 00E7F07C has view 00000000
****** Container 00E7F124 has view 02AE87D0

Stack trace:

nsContainerFrame::Destroy(nsContainerFrame * const 0x00e7f124, nsIPresContext * 
0x02a8c420) line 92 + 11 bytes
nsBoxFrame::Destroy(nsBoxFrame * const 0x00e7f124, nsIPresContext * 0x02a8c420) 
line 1016 + 13 bytes
nsFrameList::DestroyFrames(nsIPresContext * 0x02a8c420) line 36
nsContainerFrame::Destroy(nsContainerFrame * const 0x00e7f07c, nsIPresContext * 
0x02a8c420) line 99
nsBoxFrame::Destroy(nsBoxFrame * const 0x00e7f07c, nsIPresContext * 0x02a8c420) 
line 1016 + 13 bytes
nsGfxScrollFrame::Destroy(nsGfxScrollFrame * const 0x00e7f07c, nsIPresContext * 
0x02a8c420) line 452
nsFrameList::DestroyFrames(nsIPresContext * 0x02a8c420) line 36
nsContainerFrame::Destroy(nsContainerFrame * const 0x00e7ef60, nsIPresContext * 
0x02a8c420) line 99
nsBoxFrame::Destroy(nsBoxFrame * const 0x00e7ef60, nsIPresContext * 0x02a8c420) 
line 1016 + 13 bytes
nsGfxTextControlFrame2::Destroy(nsGfxTextControlFrame2 * const 0x00e7ef60, 
nsIPresContext * 0x02a8c420) line 1153 + 13 bytes
nsLineBox::DeleteLineList(nsIPresContext * 0x02a8c420, nsLineBox * 0x00e7f294) 
line 252
nsBlockFrame::Destroy(nsBlockFrame * const 0x00e7eef8, nsIPresContext * 
0x02a8c420) line 1230 + 16 bytes
nsLineBox::DeleteLineList(nsIPresContext * 0x02a8c420, nsLineBox * 0x00e7f308) 
line 252
nsBlockFrame::Destroy(nsBlockFrame * const 0x00e7f2bc, nsIPresContext * 
0x02a8c420) line 1230 + 16 bytes
nsLineBox::DeleteLineList(nsIPresContext * 0x02a8c420, nsLineBox * 0x00e7f36c) 
line 252
nsBlockFrame::Destroy(nsBlockFrame * const 0x00e6b42c, nsIPresContext * 
0x02a8c420) line 1230 + 16 bytes
nsLineBox::DeleteLineList(nsIPresContext * 0x02a8c420, nsLineBox * 0x00e6b478) 
line 252
nsBlockFrame::Destroy(nsBlockFrame * const 0x00e6b3e0, nsIPresContext * 
0x02a8c420) line 1230 + 16 bytes
nsFrameList::DestroyFrames(nsIPresContext * 0x02a8c420) line 36
nsContainerFrame::Destroy(nsContainerFrame * const 0x00e6a55c, nsIPresContext * 
0x02a8c420) line 99
nsFrameList::DestroyFrames(nsIPresContext * 0x02a8c420) line 36
nsContainerFrame::Destroy(nsContainerFrame * const 0x00e6a63c, nsIPresContext * 
0x02a8c420) line 99
nsBoxFrame::Destroy(nsBoxFrame * const 0x00e6a63c, nsIPresContext * 0x02a8c420) 
line 1016 + 13 bytes
nsFrameList::DestroyFrames(nsIPresContext * 0x02a8c420) line 36
nsContainerFrame::Destroy(nsContainerFrame * const 0x00e6a594, nsIPresContext * 
0x02a8c420) line 99
nsBoxFrame::Destroy(nsBoxFrame * const 0x00e6a594, nsIPresContext * 0x02a8c420) 
line 1016 + 13 bytes
nsGfxScrollFrame::Destroy(nsGfxScrollFrame * const 0x00e6a594, nsIPresContext * 
0x02a8c420) line 452
nsFrameList::DestroyFrames(nsIPresContext * 0x02a8c420) line 36
nsContainerFrame::Destroy(nsContainerFrame * const 0x00e6a520, nsIPresContext * 
0x02a8c420) line 99
ViewportFrame::Destroy(ViewportFrame * const 0x00e6a520, nsIPresContext * 
0x02a8c420) line 144
FrameManager::~FrameManager() line 405
FrameManager::`scalar deleting destructor'(unsigned int 1) + 15 bytes
FrameManager::Release(FrameManager * const 0x02a8ee90) line 384 + 157 bytes
PresShell::~PresShell() line 1327 + 36 bytes
PresShell::`scalar deleting destructor'() + 15 bytes
PresShell::Release(PresShell * const 0x02a8d760) line 1237 + 158 bytes
nsCOMPtr<nsIPresShell>::~nsCOMPtr<nsIPresShell>() line 490
DocumentViewerImpl::~DocumentViewerImpl() line 447 + 97 bytes
DocumentViewerImpl::`scalar deleting destructor'(unsigned int 1) + 15 bytes
DocumentViewerImpl::Release(DocumentViewerImpl * const 0x02a8bf20) line 355 + 
154 bytes
nsCOMPtr<nsIContentViewer>::assign_assuming_AddRef(nsIContentViewer * 
0x00000000) line 472
nsCOMPtr<nsIContentViewer>::assign_with_AddRef(nsISupports * 0x00000000) line 
849
nsCOMPtr<nsIContentViewer>::operator=(nsIContentViewer * 0x00000000) line 584
nsDocShell::SetupNewViewer(nsDocShell * const 0x02a6ab00, nsIContentViewer * 
0x02aec3c0) line 2854
nsWebShell::SetupNewViewer(nsWebShell * const 0x02a6ab00, nsIContentViewer * 
0x02aec3c0) line 350 + 13 bytes
nsDocShell::Embed(nsDocShell * const 0x02a6ab20, nsIContentViewer * 0x02aec3c0, 
const char * 0x01eb06ac, nsISupports * 0x00000000) line 2497 + 23 bytes
nsWebShell::Embed(nsWebShell * const 0x02a6ab20, nsIContentViewer * 0x02aec3c0, 
const char * 0x01eb06ac, nsISupports * 0x00000000) line 383
nsDocShell::CreateContentViewer(nsDocShell * const 0x02a6ab00, const char * 
0x0012fd94, nsIChannel * 0x02aecd30, nsIStreamListener * * 0x0012fde8) line 2678 
+ 32 bytes
nsDSURIContentListener::DoContent(nsDSURIContentListener * const 0x02a6a760, 
const char * 0x0012fd94, int 0, const char * 0x002d66c8 gCommonEmptyBuffer, 
nsIChannel * 0x02aecd30, nsIStreamListener * * 0x0012fde8, int * 0x0012fd78) 
line 103 + 33 bytes
nsDocumentOpenInfo::DispatchContent(nsIChannel * 0x02aecd30, nsISupports * 
0x00000000) line 359 + 109 bytes
nsDocumentOpenInfo::OnStartRequest(nsDocumentOpenInfo * const 0x02aeccc0, 
nsIChannel * 0x02aecd30, nsISupports * 0x00000000) line 233 + 16 bytes
nsStreamIOChannel::OnStartRequest(nsStreamIOChannel * const 0x02aecd34, 
nsIChannel * 0x02aecbb0, nsISupports * 0x00000000) line 611
nsOnStartRequestEvent::HandleEvent(nsOnStartRequestEvent * const 0x02aea490) 
line 210 + 26 bytes
nsStreamListenerEvent::HandlePLEvent(PLEvent * 0x02aebaf0) line 97 + 12 bytes
PL_HandleEvent(PLEvent * 0x02aebaf0) line 576 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x0177ed90) line 509 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x00120c12, unsigned int 49438, unsigned int 0, 
long 24636816) line 1054 + 9 bytes
USER32! 77e71820()
0177ed

Comment 20

[rods (gone)]

Taking a guess.... The views in the main window are being destroyed before the 
GfxTextControl frame and it's views are destroyed. because of the "position" the 
GfxText's view hierarchy references the part of the view hierarchy that is being 
destroyed.

Here is an additional example that crashes that is famirly close to the previous 
testcase:

<script> 
function destroy() { 
  self.frame1.location="about:blank";
} 
</script>

<body onload="destroy();">
  <iframe name="frame1" src="javascript:'<html><body><span 
style=&quot;position:relative&quot;><form><input></form></span></body></html>'" 
/>
</body>


the crash has to do with the "position:relative" (but then we know that), 
because if I change it to position:absolute" it works fine. So I know this is 
obvious, but what is the difference in the GfxText's view creation code paths 
between being created releative or absolute?

Comment 21

[Eric Vaughan]

I also noticed that this only happens for textfield all other form control seem
to work. I wonder if enderlite is doing anything weird with its view hierarchy.
CC mjudge

Comment 22

[Eric Vaughan]

Looks like this happens because the nsPositionedInlineFrame somehow ends up 
sharing the same view as nsScrollPortFrame view (Which is illegal). I think its 
because the relative positioning code is making a mistake when shuffeling its 
children around into different lists. 

Digging a little deeper to see what I can find.

Comment 23

[Eric Vaughan]

Ok I figured this out. The problem is the frames are moved but the views are 
not. The code was written to move the views but it was never turned on because 
the author never saw a case where you had view to move. But if you place a 
scrollframe inside it does have views that must be moved. Turning the move code 
on fixes the this bug quite nicely. Patch is attached below.

Comment 24

[Eric Vaughan]

Attachment: Fix

Comment 25

[Pierre Saslawsky]

It's buster's code. CCd him for review.

Comment 26

[buster]

r=buster.  I suggest waterson for the sr.  Please fix up the comment, and refer
to the bug number. And remember to send a note to [e-mail].
Good job fixing this.

Comment 27

[Eric Vaughan]

Ok this has been tested on win, linux, and mac. Looks like this code only gets 
executed when something is fixed:relative. So is pretty safe because 
position:relative is not incredibly common. But this should be fixed asap 
because it CRASHES the browser. 

Just waiting for watersons SR

Comment 28

[Roy Yokoyama]

Tested on my W2K-JA machine. No longer crashes after applying the patch.

Comment 29

[Chris Waterson]

Yeah, don't forget to fix up the comment! sr=waterson

Comment 30

[Jason Eager]

We have a r= and a sr=. This should get bumped up to rtm+ immediately.

PDT: We REALLY need to get this fix in. NS6 crashing on common css such as
position:relative will make it seem that Netscape 6 can STILL not handle css
properly.

Comment 31

[Eric Vaughan]

Checked into trunk.

Comment 32

[Jason Eager]

This was just moved to rtm+ by someone who didn't leave a comment. :P

Re-adding r=buster, sr=waterson. I would keep them in the status whiteboard
until we get rtm++ (or until we close this bug) so we can differentiate bugs
that are in rtm need info because they haven't gotten review yet, or if the pdt
has moved it back to need info because they have a question.

Besides, if the patch has regressions, then we can go back to the reviewers for
help. :P :-)

Comment 33

[Jim Roskind]

Please let this cook on the trunk for 24 hours, and then renominate with info on
regressions if any.  My *guess* is that we'll take this into our limbo, and
accept into N6 RTM it if can nail a candidate build RSN.

Based on discussion with Waterson, this is a very uncommon situation (a block
level item inside an inline), and the code is effectively defensive and rarely used.

Others can comment if the risk is higher or lower, or the benefit different than
percieved.

Waterson also suggested that he could propose a way to exercise and test this
code... so it would be good to have some data on the trunk build based on such
exercise.

Comment 34

[Chris Waterson]

Well, what I *think* would be uncommon is someone doing a block-in-inline (which
is not "allowed" by CSS), and then trying to make use of fancy CSS positioning
features.

Comment 35

[John Morrison]

Well, this patch fixes the crash for the testcases in this bug report 
(mac/linux/win32 trunkbuilds for today), and also some other form pages
that I modified to do a bunch of position:relative. It was also seeming 
to pass most of the regression tests under layout/html/tests/block.

However, the code that is called now will crash on the bug test
http://lxr.mozilla.org/seamonkey/source/layout/html/tests/block/bugs/22037.html
which deals with <object> contained within a <span> inside a P block.
That however may be an even more rare condition than putting position:relative
on a form field. 

Comment 36

[John Morrison]

With the exception of the crash for 22037.html, the regression tests 
under \block all claim to pass the tests, with the exception of 

  layout\html\tests\block\bugs\4468.html
  layout\html\tests\block\bugs\13940.html (which should be named 12940.html
                                           since it comes from bug 12940)

However, these claim to fail even when I revert the change. On visual 
inspection, these test cases seem to look correct. waterson/buster, though 
can make a better determination.

Comment 37

[Chris Waterson]

jrgm: hrm, bug 22037 actually turns out to be a fairly common case (I think
there were a couple of dups or near-dups), because people want to put shiny
dancing text if they can't load their plugin.

Does applying this change make the test case for 22037 break "as is", or did you
have to add "position: relative" to the test case to make it crash? If the
former, we should dig deeper before putting this into the branch. If the latter,
ship it. ;-)

Comment 38

[John Morrison]

waterson: yeah, I didn't really think it through on my initial comment. It 
wouldn't be an uncommon case.

As for what crashed: I just ran the testcase "as-is", and did _not_ modify it 
to add 'position:relative'. So deeper digging it is.

(By the way, I tested with my own debug build on win2k, pulled ~6pm yesterday. 
I did not run the tests against the trunk build (they're debug-only, right)).

Comment 39

[John Morrison]

http://lxr.mozilla.org/seamonkey/source/layout/html/tests/block/bugs/22037.html
does crash 2000102608 MTrunk win32 build (just to confirm that it's not just
my debug build that will crash with this patch).

Comment 40

[Andreas Franke (gone)]

The minimal crashing fragment from the 22037 testcase seems to be something like
<object><p></object> . 

Comment 41

[Eric Vaughan]

Ok Chris this looks like this is a bug in your code. What happens
SplitToContainingBlock calls MoveChildrenTo and the destination frame is not
currently in the frame tree and has no views associated. So when MoveChildrenTo
tries to reparent the views it can't because the new frame has no views. I think
the fix might be to just insert the destination frame list into the frame tree
before the move to make sure it can move the views. 

Comment 42

[Asa Dotzler [:asa]]

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

Comment 43

[Jason Eager]

Umm..this crashes Tom's Hardware. Now it should definately get moved to rtm++.

Comment 44

[John Morrison]

To be clear, the trunk (with the call to ReparentFrameViewList uncommented)
does crash at http://www.tomshardware.com/. The branch does not (but instead
crashes on the URL/testcase reported for this bug).

Comment 45

[Andreas Franke (gone)]

Chris, are you working on this, or is position:relative dead?

> Jim Roskind wrote 2000-11-02 12:20: RTM train is leaving...

Comment 46

[Andreas Franke (gone)]

Attachment: ugly bandaid fix for the new crashes on the trunk

Comment 47

[Andreas Franke (gone)]

With this patch, the crashes on http://www.tomshardware.com/index.html and
http://lxr.mozilla.org/seamonkey/source/layout/html/tests/block/bugs/22037.html
disappear. The pages display fine, as far as I can tell (though I must admit
that I have no idea what's going on).
Added "patch" and "review" keywords for you to remove... :)

Comment 48

[John Morrison]

Just a note: that patch does stop the bug 22037/tomshardware crash in a trunk 
build pulled 2am this morning, and passes all the block regressions tests 
except for bug 13940 (12940) and bug 5418. (But whether it's the right thing to 
do is for someone else to say. buster?).

Comment 49

[Chris Waterson]

As evaughan noted, the problem is with SplitToContainingBlock(). I'm looking at it.

Comment 50

[Andreas Franke (gone)]

Ok, forget my hack then. Nominating for NS 6.01. Is this worth a release-note?

Comment 51

[Jim Roskind]

I was just asked to look at this bug and patch.  Note that the second set of
null checks is seemingly too far away from the site at which the pointers are
dereferenced.  To be specific, if *both* parent pointers are null, the existing
code would not derefenece either of them, and would return NS_OK.
With the new patch, the code would instead return an error when both parents
became null at the same level.

I'd suggest putting the bullet proofing code (re: null checks) closer to the
site that you are trying to protect, so that semantics are not changed (unless
you want to argue that the semantics need to be changed, or that other
circumstances prevent the case I mentioned).  Even if you can argue that the
case can't happen, the patch becomes clearly a case of bullet proofing (with no
additional proof) if the second set of null checks is moved to after the parent
equality test.

Comment 52

[Jim Roskind]

If possible, please move the null checks closer to the crash site in the patch,
and acquire reviews and approvals.  If this is all done RSN, there is a chance
the the purely defensive null pointer check can make it in as a "hitch hiker" on
a forced respin of our candidate.
Marking need info to encourage motion.
Please market rtm plus asap...  

Comment 53

[Andreas Franke (gone)]

jar: the problem is that the "ugly bandaid fix for the new crashes on the trunk"
implies that the previous "Fix" has been applied (which is currently the case
for the trunk, but not for the branch). 
The "bandaid fix" just fixes the observed regressions caused by the original
"Fix", and any proof that a checkin to the branch is free of risk would have to
consider both. I'm sure that a proof could be done for a modified "bandaid fix",
but I'm not so sure about the original "Fix" (which got reviewed and
super-reviewed, but caused the new crash on the trunk).

The only reasons to consider this for checkin on the branch would be:
- the benefit that it makes the way free for use of position:relative on the web
- the knowledge that this does not regress anything covered by existing tests
- the knowledge that AFAIK the only newly reported crash on the trunk appeared
to be a duplicate of the problem already known from the regression tests, and
fixed by the second patch

Comment 54

[Andreas Franke (gone)]

Attachment: alternative hack to stop the new crashes

Comment 55

[Chris Waterson]

Either of andreas' patches look fine to me; r=waterson. jar prefers the second
patch as it's patently clear to be very safe. We would leave this bug open until
the real fix is worked out.

Comment 56

[Jon Granrose]

rtm++ per discussion with jar, selmer, and phil.

Comment 57

[Chris Waterson]

Ok, although the patch fixes the crash when the page is opened, we now crash as
soon as the page is *left* :-(. Here's the stack trace...

(gdb) bt 10
#0  0x84a5f6c in ?? ()
#1  0x419c3e03 in nsBoxFrame::Destroy (this=0x86b7710, aPresContext=0x845a488)
    at nsBoxFrame.cpp:1002
#2  0x41a367a3 in nsFrameList::DestroyFrames (this=0x86b76a0,
    aPresContext=0x845a488) at nsFrameList.cpp:35
#3  0x4172987d in nsContainerFrame::Destroy (this=0x86b766c,
    aPresContext=0x845a488) at nsContainerFrame.cpp:95
#4  0x419c3e03 in nsBoxFrame::Destroy (this=0x86b766c, aPresContext=0x845a488)
    at nsBoxFrame.cpp:1002
#5  0x4178db0d in nsGfxScrollFrame::Destroy (this=0x86b7668,
    aPresContext=0x845a488) at nsGfxScrollFrame.cpp:451
#6  0x41a367a3 in nsFrameList::DestroyFrames (this=0x86b7580,
    aPresContext=0x845a488) at nsFrameList.cpp:35
#7  0x4172987d in nsContainerFrame::Destroy (this=0x86b754c,
    aPresContext=0x845a488) at nsContainerFrame.cpp:95
#8  0x419c3e03 in nsBoxFrame::Destroy (this=0x86b754c, aPresContext=0x845a488)
    at nsBoxFrame.cpp:1002
#9  0x418c06ee in nsGfxTextControlFrame2::Destroy (this=0x86b754c,
    aPresContext=0x845a488) at nsGfxTextControlFrame2.cpp:1165
(More stack frames follow...)

Anyway, I'm demoting this to [rtm-]. We need the Real Fix.

Comment 58

[Andreas Franke (gone)]

Hey Chris, this may be a misunderstanding: The "Fix" attachment is needed to
stop the crash related to position:relative, and my hack is needed to avoid the
regression that caused. Please clarify: Have you applied the "Fix"?

Comment 59

[Chris Waterson]

We can't apply both to the branch at this point. Sorry :-(

Comment 60

[Andreas Franke (gone)]

Ah, I see. Re-adding relnote-devel.

Comment 61

[Andreas Franke (gone)]

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

Comment 62

[Andreas Franke (gone)]

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

Comment 63

[Chris Waterson]

Attachment: proposed fix; more conservative view re-parenting

Comment 64

[Chris Waterson]

buster, could you r= this? I think that the original patch (always re-parenting
views in MoveChildrenTo) was too aggressive. Specifically, when called from
SplitToContainingBlock(), I'm pretty sure that we'll *never* have a case where
an inline ends up with a view. Am I right about that?

(FWIW, SplitToContainingBlock() and ConstructInline() are the only two places
that call MoveChildrenTo().)

Comment 65

[buster]

Waterson: I can't seem to apply the patch.  Could you send me the file?

As to your question, I'm not sure, but what about this case:
<html><head>
<style>
  xxx {display:block; position: relative; left:50px;}
</style></head><body>
<span style="position: relative; left:50px;">
  text in <xxx>BLOCK</xxx>the span
</span>

Comment 66

[Chris Waterson]

File sent. I think that case will work fine, because it'll be going through the
CreateInline() codepath, which will do the view movement. The codepath that I'm
most concerned about is the replaced inline element case: we've got an inline
(e.g., an <object>) that all of a sudden becomes a block. Do we ever have view
migration problems? I guess I'll try something like this:

  <span style="position:relative">
    <object>
      <p><form><input type="text"></input></form></p>
    </object>
  </span>

That'll make it so that we've got a relatively positioned inline (which has a
view) that will need to run through the SplitToContainingBlock() code when the
<object> tag gets replaced by the <p> and its contents...

Comment 67

[buster]

Chris: your patch looks good, and fixes the 10/19/00 attached test case as well
as my test case from my comment on 2000-11-09 09:35.  But it still crashes on
the test case you added with the object tag.  Do you want to treat that as a
separate problem?  If so, submit a bug on it and sr=buster.  Otherwise, let's
work that problem before checking this in.

Comment 68

[Chris Waterson]

Yeah, I'll bang around that problem for a bit. I've got three or four other
{ib}'s that might play into this mess somehow...

Comment 69

[Chris Waterson]

Ok, seeing as I haven't really gotten 'round to trying out that other stuff,
what say I check in the patch as is, and file a new bug for the case I describe
above? I think I have sr=buster; anyone else feel comfortable reviewing the last
patch?

Comment 70

[timeless]

could you attach a patch against current cvs or any other version that will 
merge? 
based on <buster> Waterson: I can't seem to apply the patch.  Could you send me 
the file?
I'm hesitant to look at the currently attached version.  Thanks

Comment 71

[Chris Waterson]

Attachment: same fix, against r1.538 of nsCSSFrameConstructor.cpp

Comment 72

[buster]

re-iterating sr=buster, in case there was any confusion.

Comment 73

[moshev]

tomshardware is also looking quite funny with 2000122610, I think this is alsow
position:relative related. (try reading the motherboard guide).

Comment 74

[Blake Ross]

Is this patch ready to go?

Comment 75

[Chris Waterson]

Yes, pending review from nisheeth or karnaze, both who I've bugged (but may be 
on vacation). rods, maybe you could give me r= on this one?

Comment 76

[karnaze (gone)]

r=karnaze

Comment 77

[Chris Waterson]

Fixed. Opened bug 64670 to track out-of-flow inline view migration fu.

Comment 78

[Chris Waterson]

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

Comment 79

[Loco]

VERIFIED.
Mac OS 01-11-04, Win NT 01-11-04.

Also changed Plat to ALL since this issue was crashing the Mac as well.

Comment 80

[Loco]

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

Comment 81

[R.K.Aa.]

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