Closed
Bug 1170084
Opened 9 years ago
Closed 9 years ago
AccessibleCaretManager: Assertion failure: nsContentUtils::IsSafeToRunScript()
Categories
(Core :: DOM: Selection, defect)
Core
DOM: Selection
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.
Assignee | ||
Comment 1•9 years ago
|
||
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
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(bugs)
Comment 2•9 years ago
|
||
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)
Comment 3•9 years ago
|
||
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();
Assignee | ||
Comment 4•9 years ago
|
||
Olli, Thank you very much for explaining that thoroughly. I think we could use AsyncEventDispatcher for dispatching the event in this case.
Assignee | ||
Comment 5•9 years ago
|
||
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 | ||
Updated•9 years ago
|
Assignee: nobody → tlin
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•9 years ago
|
||
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 7•9 years ago
|
||
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+
Assignee | ||
Comment 8•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f10a253891dc
Keywords: checkin-needed
Comment 10•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/795725455d95
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in
before you can comment on or make changes to this bug.
Description
•