Closed
Bug 315321
Opened 19 years ago
Closed 19 years ago
nsDirectionalFrame allocated on heap
Categories
(Core :: Layout: Text and Fonts, defect)
Core
Layout: Text and Fonts
Tracking
()
RESOLVED
FIXED
People
(Reporter: roc, Assigned: mkaply)
References
Details
Attachments
(1 file)
2.15 KB,
patch
|
roc
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
nsDirectionalFrame is allocated on the heap. The obvious change to allocate it on the presshell causes crashes (see bug 315127). We need to figure out why.
Comment 1•19 years ago
|
||
If nothing else, stuff like: 310 if (nsLayoutAtoms::directionalFrame == frameType) { 311 delete frame; 312 ++lineOffset; 313 } in nsBidiPresUtils would do it, would it not? That is, this should be calling frame->Destroy(), not deleting it.... Similar code appears at 780 if (nsLayoutAtoms::directionalFrame == frame->GetType()) { 781 delete frame; 782 ++aOffset; 783 } in the same file.
http://lxr.mozilla.org/seamonkey/source/layout/base/nsBidiPresUtils.cpp#254 254 nsIFrame* frame = nsnull; 311 delete frame; The type to be deleted here is |nsIFrame*|. Since nsIFrame has no virtual destructor, the compiler resolves the functions (dtor and operator delete) statically. The compiler will not use |nsFrame::operator delete()| here.
Comment 3•19 years ago
|
||
I caused this crash originally so here's my attempt at a patch. This uses frame->Destroy as per comment #1. The testcase in bug 315127 no longer crashes and browsing around mozilla.org.il and ynet.co.il seem fine. I wasn't sure who should be reviewing this.
Attachment #202085 -
Flags: review?(roc)
Attachment #202085 -
Flags: superreview+
the patch fixes the issue in bug 315193
*** Bug 315193 has been marked as a duplicate of this bug. ***
Reporter | ||
Updated•19 years ago
|
Attachment #202085 -
Flags: review?(roc) → review+
patch checked in, Marc there is no automatic checkin when a patch gets reviewed. The system currently relies that people checkin themselfes or ask for checkin.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: zach → layout.fonts-and-text
You need to log in
before you can comment on or make changes to this bug.
Description
•