Closed
Bug 583957
Opened 14 years ago
Closed 14 years ago
"ASSERTION: killing mutation events" in nsMenuFrame::UpdateMenuType
Categories
(Core :: XUL, defect)
Tracking
()
People
(Reporter: jruderman, Assigned: smaug)
References
Details
(Keywords: assertion, testcase, Whiteboard: [sg:critical?][critsmash:patch])
Attachments
(3 files)
423 bytes,
text/html
|
Details | |
17.56 KB,
text/plain
|
Details | |
3.48 KB,
patch
|
enndeakin
:
review+
neil
:
superreview+
dbaron
:
approval2.0+
dveditz
:
approval1.9.2.11+
dveditz
:
approval1.9.1.14+
|
Details | Diff | Splinter Review |
###!!! 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.
Reporter | ||
Comment 1•14 years ago
|
||
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → Olli.Pettay
Assignee | ||
Comment 2•14 years ago
|
||
Um, nsMenuFrame::AttributeChanged is quite wrong. I blame hyatt.
Assignee | ||
Comment 3•14 years ago
|
||
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.
Assignee | ||
Updated•14 years ago
|
Attachment #462387 -
Flags: superreview?(neil)
Attachment #462387 -
Flags: review?(enndeakin)
Updated•14 years ago
|
Attachment #462387 -
Flags: superreview?(neil) → superreview+
Reporter | ||
Updated•14 years ago
|
Whiteboard: [sg:critical?]
Updated•14 years ago
|
Whiteboard: [sg:critical?] → [sg:critical?][critsmash:investigating]
Updated•14 years ago
|
Whiteboard: [sg:critical?][critsmash:investigating] → [sg:critical?][critsmash:patch]
Comment 4•14 years ago
|
||
(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.
Assignee | ||
Comment 5•14 years ago
|
||
it has if (mType != eMenuType_Radio) return and if (mType != eMenuType_Radio || ...) return
Assignee | ||
Comment 6•14 years ago
|
||
In case of eMenuType_Normal, mChecked is probably changed, but mChecked isn't really used with eMenuType_Normal.
Assignee | ||
Comment 7•14 years ago
|
||
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]
Updated•14 years ago
|
Attachment #462387 -
Flags: review?(enndeakin) → review+
Assignee | ||
Updated•14 years ago
|
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
Assignee | ||
Updated•14 years ago
|
Attachment #462387 -
Flags: approval2.0?
blocking1.9.1: ? → needed
blocking1.9.2: ? → needed
status1.9.1:
--- → wanted
status1.9.2:
--- → wanted
Attachment #462387 -
Flags: approval2.0? → approval2.0+
Assignee | ||
Updated•14 years ago
|
Whiteboard: [sg:critical?][critsmash:patch][needs review] → [sg:critical?][critsmash:patch]
Assignee | ||
Comment 9•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/e4e653196488
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 10•14 years ago
|
||
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.
Assignee | ||
Comment 11•14 years ago
|
||
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.
Assignee | ||
Comment 12•14 years ago
|
||
I need to test whether the patch applies to branches.
Updated•14 years ago
|
blocking1.9.1: needed → .13+
blocking1.9.2: needed → .10+
Assignee | ||
Updated•14 years ago
|
Attachment #462387 -
Flags: approval1.9.2.10?
Attachment #462387 -
Flags: approval1.9.1.13?
Comment 13•14 years ago
|
||
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+
Assignee | ||
Comment 14•14 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/6a2ae85c5dbc http://hg.mozilla.org/releases/mozilla-1.9.2/rev/528a466700d8
Updated•14 years ago
|
Group: core-security
Reporter | ||
Comment 15•14 years ago
|
||
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.
Description
•