Closed
Bug 243809
Opened 20 years ago
Closed 20 years ago
M17rc2 M18a1 mozilla crash at [@ nsHTMLEditor::CheckSelectionStateForAnonymousButtons]
Categories
(SeaMonkey :: Composer, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: Louie.Zhao, Unassigned)
References
Details
(Keywords: crash, fixed1.7, topcrash, Whiteboard: fixed-aviary1.0 fixed1.7)
Crash Data
Attachments
(1 file)
737 bytes,
patch
|
glazou
:
review+
jst
:
superreview+
asa
:
approval1.7+
|
Details | Diff | Splinter Review |
Step to reproduce the crash of mozilla: 1. start gnopernicus 2. start mozilla 3. open composer window Result: mozilla will crash In nsHTMLEditor::CheckSelectionStateForAnonymousButtons, "GetSelectionContainer" doesn't guarantee to return a "nsIDOMElement" successfuly. In the above case, "focusElement" is set to null by "GetSelectionContainer", which caused mozilla to crash when "focusElement->GetTagName".
Reporter | ||
Comment 1•20 years ago
|
||
Refer to http://lxr.mozilla.org/seamonkey/source/editor/libeditor/html/nsHTMLEditor.cpp#185, We need to check the result of "GetSelectionContainer" first before using it.
Comment on attachment 148654 [details] [diff] [review] patch v1 r=daniel@glazman.org
Attachment #148654 -
Flags: review+
Reporter | ||
Updated•20 years ago
|
Attachment #148654 -
Flags: superreview?(jst)
Comment 3•20 years ago
|
||
I question if not having a selection container is really an error here? Sure, we shouldn't crash, but is returning NS_ERROR_NULL_POINTER the correct thing to do here? Why not just NS_OK?
Comment 4•20 years ago
|
||
bug 212219 cri P1 All daniel@glazman.org NEW Undo + Redo of Table crashes [@ nsHTMLEditor::CheckSelect... bug 219895 cri -- All composer@editor.bugs NEW Crash on table delete row Anybody care if we DUP all the bugs to this, since we have a stack in this bug, as well as a patch?
Comment 5•20 years ago
|
||
*** Bug 219895 has been marked as a duplicate of this bug. ***
Updated•20 years ago
|
Summary: mozilla crash at nsHTMLEditor::CheckSelectionStateForAnonymousButtons → mozilla crash at [@ nsHTMLEditor::CheckSelectionStateForAnonymousButtons]
Comment 6•20 years ago
|
||
*** Bug 244089 has been marked as a duplicate of this bug. ***
Comment 7•20 years ago
|
||
*** Bug 212219 has been marked as a duplicate of this bug. ***
Comment 8•20 years ago
|
||
Adding M17rc2 and topcrash keyword for tracking (from bug 244089).
Keywords: topcrash
Summary: mozilla crash at [@ nsHTMLEditor::CheckSelectionStateForAnonymousButtons] → M17rc2 mozilla crash at [@ nsHTMLEditor::CheckSelectionStateForAnonymousButtons]
Updated•20 years ago
|
Flags: blocking1.7?
Reporter | ||
Comment 9•20 years ago
|
||
I can't see the original purpose of the return value of function "CheckSelectionStateForAnonymousButtons" because the caller doesn't use the return value at all except "EndUpdateViewBatch", which is used withouth checking return value by caller also. It's hard to judge whether NS_ERROR_NULL_POINTER or NS_OK should be used here. Based on the current codebase, it doens't matter. Maybe the original author can give us some hint.
Comment 10•20 years ago
|
||
*** Bug 241242 has been marked as a duplicate of this bug. ***
Comment 11•20 years ago
|
||
*** Bug 244092 has been marked as a duplicate of this bug. ***
Comment 12•20 years ago
|
||
sounds like I'll want this patch on the aviary branch once it gets checked in
Updated•20 years ago
|
Flags: blocking1.7? → blocking1.7+
Comment 13•20 years ago
|
||
Comment on attachment 148654 [details] [diff] [review] patch v1 @@ -203,6 +203,7 @@ nsCOMPtr<nsIDOMElement> focusElement; // let's get the containing element of the selection nsresult res = GetSelectionContainer(getter_AddRefs(focusElement)); + if (!focusElement) return NS_ERROR_NULL_POINTER; Ok, since noone spoke up with a good reason for returning an error code here, I'd say return NS_OK here. sr=jst with that change.
Attachment #148654 -
Flags: superreview?(jst) → superreview+
Sorry for the delay, I was burried under urgent matters. Yeah, NS_OK is fine too here. In fact, this code should really not been called at that time and there is something weird far above, probably in the command updater, that causes that call.
Updated•20 years ago
|
OS: Linux → All
Comment 15•20 years ago
|
||
Comment on attachment 148654 [details] [diff] [review] patch v1 this needs to get in 1.7, since it's a topcrasher
Attachment #148654 -
Flags: approval1.7?
Comment 16•20 years ago
|
||
*** Bug 244451 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 17•20 years ago
|
||
Patch checked in trunk.
Comment 19•20 years ago
|
||
Comment on attachment 148654 [details] [diff] [review] patch v1 a=asa (on behalf of drivers) for checkin to 1.7
Attachment #148654 -
Flags: approval1.7? → approval1.7+
Comment 20•20 years ago
|
||
Louie Zhao: Can you check this in to 1.7 branch then? Don't forget to add the fixed1.7 afterwards, thanks!
*** Bug 244652 has been marked as a duplicate of this bug. ***
Reporter | ||
Comment 22•20 years ago
|
||
patch checked in 1.7 Branch.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Whiteboard: fixed-aviary1.0 → fixed-aviary1.0 fixed1.7
Comment 23•20 years ago
|
||
*** Bug 245006 has been marked as a duplicate of this bug. ***
Comment 24•20 years ago
|
||
*** Bug 245187 has been marked as a duplicate of this bug. ***
*** Bug 245298 has been marked as a duplicate of this bug. ***
Comment 26•20 years ago
|
||
*** Bug 245378 has been marked as a duplicate of this bug. ***
*** Bug 245784 has been marked as a duplicate of this bug. ***
Verified FIXED on the trunk using build 2004-06-30-08 on Windows XP.
Status: RESOLVED → VERIFIED
Comment 29•20 years ago
|
||
Adding M18a1 to summary for tracking. This was a topcrasher for Mozilla 1.8 alpha 1.
Summary: M17rc2 mozilla crash at [@ nsHTMLEditor::CheckSelectionStateForAnonymousButtons] → M17rc2 M18a1 mozilla crash at [@ nsHTMLEditor::CheckSelectionStateForAnonymousButtons]
Comment 30•20 years ago
|
||
*** Bug 251478 has been marked as a duplicate of this bug. ***
Comment 31•20 years ago
|
||
*** Bug 247791 has been marked as a duplicate of this bug. ***
*** Bug 239908 has been marked as a duplicate of this bug. ***
Updated•20 years ago
|
Product: Browser → Seamonkey
Updated•13 years ago
|
Crash Signature: [@ nsHTMLEditor::CheckSelectionStateForAnonymousButtons]
You need to log in
before you can comment on or make changes to this bug.
Description
•