Crash [@nsXULTreeItemAccessible::GetRoleInternal]

RESOLVED FIXED

Status

()

Core
Disability Access APIs
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: davidb, Assigned: davidb)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 432603 [details] [diff] [review]
null check.

nsCoreUtils::GetFirstSensibleColumn can return null...

https://crash-stats.mozilla.com/report/index/9aebbb6a-1093-40a4-91fe-a5ace2100311
Attachment #432603 - Attachment is patch: true
Attachment #432603 - Attachment mime type: application/octet-stream → text/plain
Attachment #432603 - Flags: review?(surkov.alexander)
Assignee: nobody → bolterbugz

Comment 1

8 years ago
Is this because of a malformed XUL tree or what's going on?
Beats me.

Comment 3

8 years ago
Comment on attachment 432603 [details] [diff] [review]
null check.


>   nsCOMPtr<nsITreeColumn> column =
>     nsCoreUtils::GetFirstSensibleColumn(mTree);
> 
>   PRBool isPrimary = PR_FALSE;
>-  column->GetPrimary(&isPrimary);
>+
>+  if (column) {
>+    column->GetPrimary(&isPrimary);
>+  }
> 

I wonder why don't we use nsITreeColumns::getPrimaryColumn()?
(In reply to comment #3)
> (From update of attachment 432603 [details] [diff] [review])
> 
> >   nsCOMPtr<nsITreeColumn> column =
> >     nsCoreUtils::GetFirstSensibleColumn(mTree);
> > 
> >   PRBool isPrimary = PR_FALSE;
> >-  column->GetPrimary(&isPrimary);
> >+
> >+  if (column) {
> >+    column->GetPrimary(&isPrimary);
> >+  }
> > 
> 
> I wonder why don't we use nsITreeColumns::getPrimaryColumn()?

I think we should! Nice catch.
Attachment #432603 - Flags: review?(surkov.alexander)
(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 432603 [details] [diff] [review] [details])
> > 
> > >   nsCOMPtr<nsITreeColumn> column =
> > >     nsCoreUtils::GetFirstSensibleColumn(mTree);
> > > 
> > >   PRBool isPrimary = PR_FALSE;
> > >-  column->GetPrimary(&isPrimary);
> > >+
> > >+  if (column) {
> > >+    column->GetPrimary(&isPrimary);
> > >+  }
> > > 
> > 
> > I wonder why don't we use nsITreeColumns::getPrimaryColumn()?
> 
> I think we should! Nice catch.

It won't account for hidden columns though... is this a concern?
I guess I will just change nsXULTreeItemAccessible::GetRoleInternal and leave GetFirstSensibleColumn as is.
New patch will come after lunch :)

Comment 8

8 years ago
(In reply to comment #5)

> 
> It won't account for hidden columns though... is this a concern?

Yes if it can be hidden. I know it can't be hidden from UI. But it's worth to check if it can be programmatically.
Created attachment 433366 [details] [diff] [review]
use GetPrimaryFrame
Attachment #432603 - Attachment is obsolete: true
Attachment #433366 - Flags: review?(surkov.alexander)

Comment 10

8 years ago
(In reply to comment #9)
> Created an attachment (id=433366) [details]
> use GetPrimaryFrame

column? It would be nice to know if primary column can be presented and hidden. I think nsITreeColumns object should care about this but it's worth to check. Could you do this please?
(In reply to comment #10)
> (In reply to comment #9)
> > Created an attachment (id=433366) [details] [details]
> > use GetPrimaryFrame
> 
> column? It would be nice to know if primary column can be presented and hidden.
> I think nsITreeColumns object should care about this but it's worth to check.

nsITreeColumns doesn't care.

> Could you do this please?

Maybe but I'm not exactly sure what you are asking :)

Note nsXULTreeGridAccessible does the same thing I'm doing in this patch.
Ultimately the columns are looped through until IsPrimary succeeds:
PRBool IsPrimary() { return mIsPrimary; }
Aha we are okay, thanks surkov. nsITreeColumns cares about visibility in EnsureColumns.

Comment 14

8 years ago
Comment on attachment 433366 [details] [diff] [review]
use GetPrimaryFrame

r=me - nsITreeColumns cares the primary column is always visible so that's what we need.
Attachment #433366 - Flags: review?(surkov.alexander) → review+
Landed on central: http://hg.mozilla.org/mozilla-central/rev/126bdd9262b5
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.