Closed Bug 1170084 Opened 5 years ago Closed 5 years ago

AccessibleCaretManager: Assertion failure: nsContentUtils::IsSafeToRunScript()

Categories

(Core :: DOM: Selection, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Steps to reproduce:
1. Enable "layout.accessiblecaret.enabled"
2. Open https://en.m.wikipedia.org/wiki/Firefox
3. Select some text to make AccessibleCaret shown.
4. Click the image "A screenshot of Firefox 33.0.3 running within the Xfce desktop environment"

Actual result:
The tab crashes with full callstack dump here. https://pastebin.mozilla.org/8835292

Expected result:
The tab does not crash.
Hi Olli,

Could you explain further why it's needed to add MOZ_ASSERT(nsContentUtils::IsSafeToRunScript()) in [1] as you suggested in bug 1155493 comment 9? We now hit this assert. Thanks.

https://hg.mozilla.org/mozilla-central/file/9eae3880b132/layout/base/AccessibleCaretManager.cpp#l892
Flags: needinfo?(bugs)
If you're dispatching an event, the expectation is that some listener may handle it, and
that listener is possibly implemented in JS. However if it is not safe to run scripts, we shouldn't be in a state where we try to run scripts (the event listener).

So, something on the stack has explicitly used nsAutoScriptBlocker or similar to indicate that it is
very much unsafe to run any scripts (running scripts when it it not safe to run them may cause crashes, security issues and what not).

So, check where the script blocker is used and why.
Flags: needinfo?(bugs)
Looks like the script blocker comes from 
http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp?rev=38a6cc715880#4544

Would it be possible to dispatch mozcaretstatechanged asynchronously using a script runner?
That would mean the event is dispatched when it is safe to dispatch it - when the last script blocker is removed from stack.
Replace doc->DispatchEvent(event, ...) with something like
(new AsyncEventDispatcher(doc, event))->RunDOMEventWhenSafe();
Olli, 

Thank you very much for explaining that thoroughly. I think we could use AsyncEventDispatcher for dispatching the event in this case.
Bug 1170084 - Dispatch CaretStateChangedEvent via AsyncEventDispatcher. r?mtseng

We should not dispatch an event if it is not safe to run script since
the event handlers might be implemented by Javascript.
To fix this, we always use AsyncEventDispatcher to dispatch the event.
Assignee: nobody → tlin
Status: NEW → ASSIGNED
Comment on attachment 8615848 [details]
MozReview Request: Bug 1170084 - Dispatch CaretStateChangedEvent via AsyncEventDispatcher. r?mtseng

Bug 1170084 - Dispatch CaretStateChangedEvent via AsyncEventDispatcher. r?mtseng

We should not dispatch an event if it is not safe to run script since
the event handlers might be implemented by Javascript.
To fix this, we always use AsyncEventDispatcher to dispatch the event.
Attachment #8615848 - Flags: review?(mtseng)
Comment on attachment 8615848 [details]
MozReview Request: Bug 1170084 - Dispatch CaretStateChangedEvent via AsyncEventDispatcher. r?mtseng

https://reviewboard.mozilla.org/r/10371/#review9081

LGTM
Attachment #8615848 - Flags: review?(mtseng) → review+
https://hg.mozilla.org/mozilla-central/rev/795725455d95
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.