Closed Bug 309599 Opened 15 years ago Closed 15 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: 15 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.