Closed Bug 583957 Opened 14 years ago Closed 14 years ago

"ASSERTION: killing mutation events" in nsMenuFrame::UpdateMenuType

Categories

(Core :: XUL, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking1.9.2 --- .11+
status1.9.2 --- .11-fixed
blocking1.9.1 --- .14+
status1.9.1 --- .14-fixed

People

(Reporter: jruderman, Assigned: smaug)

References

Details

(Keywords: assertion, testcase, Whiteboard: [sg:critical?][critsmash:patch])

Attachments

(3 files)

Attached file testcase
###!!! ASSERTION: Want to fire mutation events, but it's not safe: '(aNode->IsNodeOfType(nsINode::eCONTENT) && static_cast<nsIContent*>(aNode)-> IsInNativeAnonymousSubtree()) || sScriptBlockerCount == sRemovableScriptBlockerCount', file content/base/src/nsContentUtils.cpp, line 3619

###!!! ASSERTION: killing mutation events: 'nsContentUtils::IsSafeToRunScript()', file content/base/src/nsContentUtils.cpp, line 6142

###!!! ASSERTION: This is unsafe! Fix the caller!: 'Error', file content/events/src/nsEventDispatcher.cpp, line 514

Security-sensitive because previous bugs with these assertions (bug 557398, bug 564461) were deemed to be likely-exploitable.
Attached file assertion stack traces
Assignee: nobody → Olli.Pettay
Um, nsMenuFrame::AttributeChanged is quite wrong.
I blame hyatt.
Attached patch patchSplinter Review
I think we need this. I'll test this some more.

if (mType != eMenuType_Normal) check shouldn't be needed since 
UpdateMenuSpecialState does that too.
Attachment #462387 - Flags: superreview?(neil)
Attachment #462387 - Flags: review?(enndeakin)
Attachment #462387 - Flags: superreview?(neil) → superreview+
Whiteboard: [sg:critical?]
Whiteboard: [sg:critical?] → [sg:critical?][critsmash:investigating]
Whiteboard: [sg:critical?][critsmash:investigating] → [sg:critical?][critsmash:patch]
(In reply to comment #3)
> if (mType != eMenuType_Normal) check shouldn't be needed since 
> UpdateMenuSpecialState does that too.

I don't see UpdateMenuSpecialState checking mType.
it has
if (mType != eMenuType_Radio) return
and
if (mType != eMenuType_Radio || ...) return
In case of eMenuType_Normal, mChecked is probably changed,
but mChecked isn't really used with eMenuType_Normal.
But if really wanted I could add the pretty useless mType != eMenuType_Normal back.
Neil needs to respond.
Whiteboard: [sg:critical?][critsmash:patch] → [sg:critical?][critsmash:patch][needs review]
Attachment #462387 - Flags: review?(enndeakin) → review+
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
Attachment #462387 - Flags: approval2.0?
blocking1.9.1: ? → needed
blocking1.9.2: ? → needed
Whiteboard: [sg:critical?][critsmash:patch][needs review] → [sg:critical?][critsmash:patch]
http://hg.mozilla.org/mozilla-central/rev/e4e653196488
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Is this bug a regression, or did we just not have those assertions on the branches?

In a 1.9.2 debug build I get
###!!! ASSERTION: killing mutation events: 'nsContentUtils::IsSafeToRunScript()', file /Users/daniel/dev/ff192/content/base/src/nsContentUtils.cpp, line 5245
###!!! ASSERTION: killing mutation events: 'nsContentUtils::IsSafeToRunScript()', file /Users/daniel/dev/ff192/content/base/src/nsContentUtils.cpp, line 5245

but not the scary first and third assertion from comment 0. On the other hand, if you're blaming hyatt (comment 2) I guess this is an old problem.
We don't have all the same assertions on 1.9.2.
We should land this to branches too.
Sorry that I didn't ask approval, yet.
I need to test whether the patch applies to branches.
blocking1.9.1: needed → .13+
blocking1.9.2: needed → .10+
Attachment #462387 - Flags: approval1.9.2.10?
Attachment #462387 - Flags: approval1.9.1.13?
Comment on attachment 462387 [details] [diff] [review]
patch

Approved for 1.9.2.11 and 1.9.1.14, a=dveditz for release-drivers
Attachment #462387 - Flags: approval1.9.2.11?
Attachment #462387 - Flags: approval1.9.2.11+
Attachment #462387 - Flags: approval1.9.1.14?
Attachment #462387 - Flags: approval1.9.1.14+
Group: core-security
Crashtest: http://hg.mozilla.org/mozilla-central/rev/ea9ed8b0fe1b
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: