Last Comment Bug 654770 - "ASSERTION: Won't check if this is chrome..." with accessibility enabled
: "ASSERTION: Won't check if this is chrome..." with accessibility enabled
Status: RESOLVED FIXED
: assertion, regression
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: x86 Linux
: P1 normal (vote)
: ---
Assigned To: Olli Pettay [:smaug]
:
Mentors:
Depends on:
Blocks: 571613 613634
  Show dependency treegraph
 
Reported: 2011-05-04 11:05 PDT by Jesse Ruderman
Modified: 2011-05-10 03:46 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (910 bytes, patch)
2011-05-04 11:17 PDT, Olli Pettay [:smaug]
no flags Details | Diff | Splinter Review
patch (3.94 KB, patch)
2011-05-04 11:45 PDT, Olli Pettay [:smaug]
jonas: review+
Details | Diff | Splinter Review

Description Jesse Ruderman 2011-05-04 11:05:37 PDT
###!!! ASSERTION: Won't check if this is chrome, you want to set aWantsUntrusted to PR_FALSE or make the aWantsUntrusted explicit by making optional_argc non-zero.: '!aWantsUntrusted || optional_argc > 1', file ../../../../content/base/src/nsDocument.cpp, line 6298

nsDocument::AddEventListener [nsDocument.cpp:6300]
nsRootAccessible::AddEventListeners [nsRootAccessible.cpp:267]
nsDocAccessible::Init [nsDocAccessible.cpp:614]
nsAccDocManager::CreateDocOrRootAccessible [nsAccDocManager.cpp:456]
nsAccDocManager::GetDocAccessible [nsAccDocManager.cpp:81]
nsApplicationAccessible::CacheChildren [nsApplicationAccessible.cpp:423]
nsAccessible::EnsureChildren [nsAccessible.cpp:3163]
nsAccDocManager::GetDocAccessible [nsAccDocManager.cpp:76]
nsAccessibilityService::ContentRemoved [nsAccessibilityService.cpp:542]
nsCSSFrameConstructor::ContentRemoved [nsCSSFrameConstructor.cpp:7464]
nsCSSFrameConstructor::RecreateFramesForContent [nsCSSFrameConstructor.cpp:9109]
nsCSSFrameConstructor::ProcessRestyledFrames [nsCSSFrameConstructor.cpp:7982]
nsCSSFrameConstructor::RestyleElement [nsCSSFrameConstructor.cpp:8064]
mozilla::css::RestyleTracker::ProcessOneRestyle [RestyleTracker.cpp:156]
mozilla::css::RestyleTracker::ProcessRestyles [RestyleTracker.cpp:222]
nsCSSFrameConstructor::ProcessPendingRestyles [nsCSSFrameConstructor.cpp:11616]
PresShell::FlushPendingNotifications [nsPresShell.cpp:4791]
nsRefreshDriver::Notify [nsRefreshDriver.cpp:366]
nsTimerImpl::Fire [nsTimerImpl.cpp:428]
nsTimerEvent::Run [nsTimerImpl.cpp:522]
nsThread::ProcessNextEvent [nsThread.cpp:618]
NS_ProcessNextEvent_P [nsThreadUtils.cpp:250]
mozilla::ipc::MessagePump::Run [MessagePump.cpp:110]
MessageLoop::RunInternal [message_loop.cc:219]
MessageLoop::RunHandler [message_loop.cc:203]
MessageLoop::Run [message_loop.cc:175]
nsBaseAppShell::Run [nsBaseAppShell.cpp:191]
nsAppStartup::Run [nsAppStartup.cpp:224]
XRE_main [nsAppRunner.cpp:3682]
main [nsBrowserApp.cpp:158]
libc.so.6 + 0x15dec

Probably a regression from mozilla-central changeset f0ad024522bb:
user:        Olli Pettay
date:        Wed May 04 17:13:28 2011 +0300
summary:     Bug 613634, make the 3rd paramater of add/removeEventListener optional, r=sicking

I noticed this bug because I hit it during fuzzing, but it also happens on Tinderbox (without turning Tinderbox orange, see bug 571613).

Before (does not hit the assertion):
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1304521011.1304524426.15516.gz&fulltext=1

After (hits the assertion):
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1304526794.1304530259.17266.gz&fulltext=1
Comment 1 Jesse Ruderman 2011-05-04 11:08:30 PDT
Btw, the assertion condition and message no longer match.
Comment 2 Olli Pettay [:smaug] 2011-05-04 11:14:36 PDT
Uh, I need to go through even more code.

AddEventListener(NS_ConvertASCIItoUTF16(*e), this, PR_TRUE, PR_TRUE, 1);
should now pass 2 as the last parameter.
Comment 3 Olli Pettay [:smaug] 2011-05-04 11:17:19 PDT
Created attachment 530090 [details] [diff] [review]
patch

I don't know how to test this - I mean I don't even know how to get
the assertion.
Comment 4 Olli Pettay [:smaug] 2011-05-04 11:27:40 PDT
Comment on attachment 530090 [details] [diff] [review]
patch

Actually, I'll fix also other cases in this bug.
Comment 5 Olli Pettay [:smaug] 2011-05-04 11:45:54 PDT
Created attachment 530104 [details] [diff] [review]
patch

I'll fix the assertion text elsewhere.

The a11y case is the only one which may change the behavior (back to what it was).
Comment 6 David Bolter [:davidb] 2011-05-04 11:52:51 PDT
(In reply to comment #5)
> Created attachment 530104 [details] [diff] [review] [review]

> The a11y case is the only one which may change the behavior (back to what it
> was).

Can you explain this a bit more?
Comment 7 Olli Pettay [:smaug] 2011-05-04 12:22:41 PDT
Oh, actually, since it a11y passes PR_TRUE, I didn't break even that.
Bug 613634 just causes the assertion.

Btw, why does a11y listen for untrusted events?
Comment 8 Olli Pettay [:smaug] 2011-05-05 14:29:52 PDT
http://hg.mozilla.org/mozilla-central/rev/8ccb9dff65d7
Comment 9 alexander :surkov 2011-05-10 03:46:20 PDT
(In reply to comment #7)
> Oh, actually, since it a11y passes PR_TRUE, I didn't break even that.
> Bug 613634 just causes the assertion.
> 
> Btw, why does a11y listen for untrusted events?

XBL bindings fires events for us like XUL radio fires RadioStateChange event. Based on that we fire a11y events. If I get right these are untrusted events (or they were :) ).

Note You need to log in before you can comment on or make changes to this bug.