Closed
Bug 64670
Opened 24 years ago
Closed 24 years ago
{ib} replaced out-of-flow inline element with view crashes browser
Categories
(Core :: Layout, defect, P2)
Core
Layout
Tracking
()
VERIFIED
FIXED
mozilla0.9
People
(Reporter: waterson, Assigned: waterson)
Details
(Keywords: crash)
Attachments
(4 files)
113 bytes,
text/html
|
Details | |
144 bytes,
text/html
|
Details | |
3.57 KB,
patch
|
Details | Diff | Splinter Review | |
10.12 KB,
patch
|
Details | Diff | Splinter Review |
Whadda mouthful! This is a follow-up to bug 57026. Specifically, a replaced
inline element that is not in the normal flow (e.g., the <object> in a
relatively positioned <span>, below)
<span style="position:relative">
<object>
<p><form><input type="text"></input></form></p>
</object>
</span>
will crash the browser. What happens is
1. the <object> is initially created as an inline frame
2. we realize that we can't render the object
3. we replace the <object> with the <p>, converting a inline to a block
This runs through nsCSSFrameConstructor::SplitToContainingBlock(), which isn't
clever enough to migrate the <input> element's view properly.
Assignee | ||
Comment 1•24 years ago
|
||
Assignee | ||
Updated•24 years ago
|
Assignee | ||
Comment 2•24 years ago
|
||
Assignee | ||
Comment 3•24 years ago
|
||
Assignee | ||
Comment 4•24 years ago
|
||
Moving to mozilla0.9: this case is now *extremely* rare. Might become more
important if we get rid of WipeContainingBlock() in favor of
SplitToContainingBlock().
Target Milestone: mozilla0.8 → mozilla0.9
Assignee | ||
Updated•24 years ago
|
Summary: replaced out-of-flow inline element with view crashes browser → {ib} replaced out-of-flow inline element with view crashes browser
Assignee | ||
Comment 5•24 years ago
|
||
Assignee | ||
Comment 6•24 years ago
|
||
The above patch fixes the following:
1. A comma where there should've been a semi-colon. (Ack! C rules!)
2. I was mixing up the replaced frame and its parent when invoking
SplitToContainingBlock(). This is the third and fifth chunks in
nsCSSFrameConstructor.cpp
3. SplitToContainingBlock() now sanity checks its arguments, and handles
cases where aLeftInlineChild or aRightInlineChild may be null.
4. SplitToContainingBlock() now is aware of frames that have views, and
reparents accordingly.
5. Fix nsObjectFrame to only stifle key events if the plugin instance was
successfully instantiated (otherwise, you can't replace the object frame
with an <input>, e.g.!) saari, double-check me on that one...
Fishing for some r= lovin'...
Comment 7•24 years ago
|
||
Chris, thanks for the mondo-comment on SplitToContainingBlock() - it is very
clear but, uh, kinda complicated (I didn't know you were such the ASCII-artist).
I think it is interesting that we never check if parentFrame is null in
CantRenderReplacedElement - at least an assertion? My only other comment is that
the indenting in the object frame stuff appears to be wrong (you added a new
if() but did not indent what now is inside that block). [s]r=attinasi
Assignee | ||
Comment 8•24 years ago
|
||
Thanks for looking at this. Coupla questions...
- I copied the comment from way back when I was figuring this out. Is it
too gaudy? I could extract it and just refer to it if that's better.
- I'll add a check for null pointer; I think this case is safe because of
other, earlier out-of-memory conditions, but what the heck.
- It's a ``diff -wu'', so whitespace differences aren't shown. I can give you
the full-on diff if that's better...
Comment 9•24 years ago
|
||
Chris, the big ol' honkin' comment is fine, it is just the concept that is
complicated I think. But I actually like that the details are right there with
the code. The null ptr check will be good, or as assertion (can a frame have a
null parent in that context? - I doubt it, so probably just assert). Finally,
the diff is fine, I didn't realize that whitespace changes could be ignored like
that - that is a sweet option. Thanks for the replies.
Comment 10•24 years ago
|
||
The view creation/reparenting logic in the patch looks fine to me.
r=kmcclusk@netscape.com
Comment 11•24 years ago
|
||
The objectframe changes look good to me.
Assignee | ||
Comment 12•24 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•