Closed Bug 309599 Opened 19 years ago Closed 19 years ago

crash in nsIHTMLEditor::insertHTML

Categories

(Core :: DOM: Editor, defect)

1.8 Branch
x86
Windows 2000
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: surkov, Assigned: martijn.martijn)

References

Details

(4 keywords)

Attachments

(3 files)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4 nsIHTMLEditor.insertHTML("") crashes mozilla. Reproducible: Always
Attached file testcase
The insertHTML dialog for Composer does a null-check for this case, see http://lxr.mozilla.org/mozilla/source/editor/ui/dialogs/content/EdInsSrc.js#72. If you mess around with interfaces which directly interact with the engine, i think you should do things like null-check yourself...
(In reply to comment #2) > The insertHTML dialog for Composer does a null-check for this case, see > http://lxr.mozilla.org/mozilla/source/editor/ui/dialogs/content/EdInsSrc.js#72. > If you mess around with interfaces which directly interact with the engine, i > think you should do things like null-check yourself... When I got a crash then I begun to check incorrect values. But I belive mozilla should not be crashed in any case.
Attached file testcase2
Same as previous testcase, but with enableprivilege for easy testing the crash.
According to bug 309459, comment 4: "Crashing is always a bug. In this case, visitEntries should be null-checking its argument." So that would make this a genuine bug, not?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: testcase
Blocks: 309731
Attached patch patchSplinter Review
This fixes it for me.
Attachment #215754 - Flags: review?(neil)
Assignee: mozeditor → martijn.martijn
Not sure, but I guess some code in EdInsSrc.js could be removed also, if the patch is fine.
Blocks: 331275
Comment on attachment 215754 [details] [diff] [review] patch Sorry, I don't really understand this code; try brade or glazou perhaps?
Attachment #215754 - Flags: review?(neil)
Attachment #215754 - Flags: review?(brade)
Comment on attachment 215754 [details] [diff] [review] patch r=brade
Attachment #215754 - Flags: review?(brade) → review+
Attachment #215754 - Flags: superreview?(jst)
Comment on attachment 215754 [details] [diff] [review] patch sr=jst (and please land on the 1.8.1 branch too)
Attachment #215754 - Flags: superreview?(jst)
Attachment #215754 - Flags: superreview+
Attachment #215754 - Flags: approval-branch-1.8.1+
Checking in editor/libeditor/html/nsHTMLDataTransfer.cpp; /cvsroot/mozilla/editor/libeditor/html/nsHTMLDataTransfer.cpp,v <-- nsHTMLData Transfer.cpp new revision: 1.115; previous revision: 1.114 done Fixed on trunk.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Checked in on the 1.8 branch. mozilla/editor/libeditor/html/nsHTMLDataTransfer.cpp; new revision: 1.110.2.2;
OS: Windows 2000 → All
Hardware: PC → All
Target Milestone: --- → mozilla1.8.1
Johnny, should I ask for approval 1.8.0.3? Bug 331275 is a recent regression in the 1.8 branch, that gets fixed by this patch. See bug 331275, comment 4, 306 crashes found in the old database for Thunderbird. I think the patch is safe, not?
OS: All → Windows 2000
Hardware: All → PC
Target Milestone: mozilla1.8.1 → ---
Attachment #215754 - Flags: approval1.8.0.3?
Comment on attachment 215754 [details] [diff] [review] patch Please check in promptly on the 1.8.0 branch. Thanks!
Attachment #215754 - Flags: approval1.8.0.3? → approval1.8.0.3+
Checked in on the 1.8.0 branch. mozilla/editor/libeditor/html/nsHTMLDataTransfer.cpp 1.110.2.1.4.1
Keywords: fixed1.8.0.4
Version: Trunk → 1.8 Branch
verified using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4. No crash using either testcase.
Keywords: crash
*** Bug 334883 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: