Closed
Bug 1034978
Opened 10 years ago
Closed 8 years ago
crash in nsIContent::GetPrimaryFrame() called from mozilla::a11y::TextAttrsMgr::BGColorTextAttr::GetValueFor(mozilla::a11y::Accessible *,unsigned int *)
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
mozilla49
People
(Reporter: MarcoZ, Assigned: surkov)
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file, 1 obsolete file)
8.55 KB,
patch
|
MarcoZ
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-63bc6a23-5853-4b6d-90bf-cc6b22140706. ============================================================= This is probably related to, but not quite the same, as bug 819291. I was doing the following: 1. Visit http://www.tagesschau.de/inland/nsa-ausschuss-104.html 2. In the video's iframe, click on Settings, and make sure the player is set to HTML, not Flash. 3. Run NVDA. 4. Hit the Play button, and then alt-tab away from Firefox to not hear the ascending beeps from NVDA's progress bar indicator. The crash occurred right after the video had finished playing. The frames in this crash report point to the elm variable possibly being NULL here: http://hg.mozilla.org/mozilla-central/annotate/81691a55e60f/accessible/base/TextAttrs.cpp#l343 Alex, would a simple null check here be the appropriate fix?
Flags: needinfo?(surkov.alexander)
Assignee | ||
Comment 1•10 years ago
|
||
I'd say so however I don't have good ideas what may be a cause. The accessible has to be a text leaf accessilbe which should always have a content and parent element. Perhaps it's a right place to add a check into TTextAttr::Equal.
Flags: needinfo?(surkov.alexander)
Reporter | ||
Comment 2•10 years ago
|
||
(In reply to alexander :surkov from comment #1) > I'd say so however I don't have good ideas what may be a cause. The > accessible has to be a text leaf accessilbe which should always have a > content and parent element. Perhaps it's a right place to add a check into > TTextAttr::Equal. Hm, not sure what that would look like, or rather, what I should be comparing to what.
Flags: needinfo?(surkov.alexander)
Reporter | ||
Comment 3•10 years ago
|
||
OK, this is happening more often, and it appears to be something also involving plugins, since PluginContainer also crashes, but I have no reports of those in my about:crashes. Here's my latest report from just a few minutes ago: https://crash-stats.mozilla.com/report/index/4a533f26-6150-4a6d-a96c-514c92140711
Reporter | ||
Comment 4•10 years ago
|
||
This is a regression, since it doesn't crash with the same steps in the Firefox 30 release. What happens once I press Enter on the Users clickable in the Google Apps admin console is that focus is put onto a WAI-ARIA TreeView with filtering options, once the new content has finished loading.
Flags: needinfo?(surkov.alexander)
Keywords: regression
Reporter | ||
Comment 5•10 years ago
|
||
Sorry, cleared the needinfo request by accident.
Flags: needinfo?(surkov.alexander)
Reporter | ||
Updated•10 years ago
|
Summary: crash in nsIContent::GetPrimaryFrame() → crash in nsIContent::GetPrimaryFrame() called from mozilla::a11y::TextAttrsMgr::BGColorTextAttr::GetValueFor(mozilla::a11y::Accessible *,unsigned int *)
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Marco Zehe (:MarcoZ) from comment #2) > (In reply to alexander :surkov from comment #1) > > I'd say so however I don't have good ideas what may be a cause. The > > accessible has to be a text leaf accessilbe which should always have a > > content and parent element. Perhaps it's a right place to add a check into > > TTextAttr::Equal. > > Hm, not sure what that would look like, or rather, what I should be > comparing to what. no luck to reproduce it. I would try to add aAccessible->GetContent() check and assertion into TTextAttr::Equal right before GetValueFor call. If problems goes away then it is. Otherwise I would check result of nsCoreUtils::GetDOMElementFor on null
Flags: needinfo?(surkov.alexander)
Updated•9 years ago
|
Crash Signature: [@ nsIContent::GetPrimaryFrame()] → [@ nsIContent::GetPrimaryFrame()]
[@ nsIContent::GetPrimaryFrame]
There have been a bunch of these crashes on Aurora 48.0a2 in the last few days, although from just 2 users (one of them in 2 different nightlies). I took a look at bp-7a47279b-fcfe-436e-a645-d3ddb2160430 , which was one of the crashes submitted, and looked at the minidump, which confirmed that the crash is because nsIContent::GetPrimaryFrame is being called with this=0x0, i.e., because elm is null in the frame above.
Assignee | ||
Comment 8•8 years ago
|
||
Attachment #8747790 -
Flags: review?(mzehe)
Assignee | ||
Comment 9•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b13bc8a56bc
Assignee | ||
Comment 10•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=229967828cce
Assignee | ||
Comment 11•8 years ago
|
||
Attachment #8747790 -
Attachment is obsolete: true
Attachment #8747790 -
Flags: review?(mzehe)
Attachment #8747796 -
Flags: review?(mzehe)
Reporter | ||
Updated•8 years ago
|
Attachment #8747796 -
Flags: review?(mzehe) → review+
Reporter | ||
Comment 12•8 years ago
|
||
Setting Affected Flags in accordance with comment #7. Make sure to get this into Aurora when it lands on Central.
status-firefox48:
--- → affected
status-firefox49:
--- → affected
Assignee | ||
Comment 13•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b24c4b57e98
Assignee | ||
Comment 14•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b77b09e1e42037a34e9fbcfe1a39b3ec7f59e11d Bug 1034978 - crash in text attributes computation, r=marcoz
Comment 15•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b77b09e1e420
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment 16•8 years ago
|
||
i have also similar problems, https://crash-stats.mozilla.com/report/index/bp-57eb4ddb-bbac-4f2f-98a5-3a9972160423 https://crash-stats.mozilla.com/report/index/bp-e7b81f74-7866-431a-8c02-8b47e2160401 https://crash-stats.mozilla.com/report/index/bp-79e60710-e75d-4466-9f4a-94abd2160306 https://crash-stats.mozilla.com/report/index/bp-79b50511-facd-4328-8f2f-5d0122160226 https://crash-stats.mozilla.com/report/index/bp-022857d7-25db-43a1-9ccb-97f562160109
Comment 17•8 years ago
|
||
Hi Alexander, Since this patch also affects 48, are you also considering to uplift this patch to 48?
Flags: needinfo?(surkov.alexander)
Assignee | ||
Comment 18•8 years ago
|
||
Comment on attachment 8747796 [details] [diff] [review] patch + assertions Approval Request Comment [Feature/regressing bug #]:unknown, may be an old issue [User impact if declined]:crashes [Describe test coverage new/current, TreeHerder]:no automated test coverage, but no crashes on 49 anymore [Risks and why]: low, bunch of null checks [String/UUID change made/needed]:no
Flags: needinfo?(surkov.alexander)
Attachment #8747796 -
Flags: approval-mozilla-aurora?
Comment 19•8 years ago
|
||
Comment on attachment 8747796 [details] [diff] [review] patch + assertions Fix a crash, taking it
Attachment #8747796 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 20•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/8f2c9241267a
Updated•8 years ago
|
Assignee: nobody → surkov.alexander
You need to log in
before you can comment on or make changes to this bug.
Description
•