Closed Bug 315321 Opened 19 years ago Closed 19 years ago

nsDirectionalFrame allocated on heap

Categories

(Core :: Layout: Text and Fonts, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: roc, Assigned: mkaply)

References

Details

Attachments

(1 file)

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.
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.
Blocks: 315193
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.
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)
the patch fixes the issue in bug 315193
*** Bug 315193 has been marked as a duplicate of this bug. ***
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.

Attachment

General

Creator:
Created:
Updated:
Size: