WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
27011
[Gtk] Implement support for get_index_in_parent
https://bugs.webkit.org/show_bug.cgi?id=27011
Summary
[Gtk] Implement support for get_index_in_parent
Joanmarie Diggs
Reported
2009-07-06 15:49:54 PDT
Steps to reproduce: 1. Launch Epiphany and navigate to google.com 2. In Accerciser's tree on the left, select/highlight the accessible link for "Maps" 3. In Accerciser's iPython Console, type: acc.getIndexInParent() Expected results: 3 Actual results: 0 Note that this bug is not limited to Google or links or whathaveyou. That's merely an example. getIndexInParent seems to always return 0 for descendants of the document frame.
http://library.gnome.org/devel/atk/stable/AtkObject.html#atk-object-get-index-in-parent
Attachments
getindexinparent.patch
(2.31 KB, patch)
2009-07-08 10:26 PDT
,
Xan Lopez
no flags
Details
Formatted Diff
Diff
test case
(426 bytes, text/html)
2009-07-08 11:42 PDT
,
Joanmarie Diggs
no flags
Details
getindexinparent.patch
(2.24 KB, patch)
2009-10-26 13:02 PDT
,
Xan Lopez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Xan Lopez
Comment 1
2009-07-08 10:26:43 PDT
Created
attachment 32456
[details]
getindexinparent.patch This implements the function, but the particular testcase you mention does not work because the object hierarchy is not quite right in Google's case (the Map object has a dummy 'label' parent).
Joanmarie Diggs
Comment 2
2009-07-08 11:42:52 PDT
Created
attachment 32463
[details]
test case Thanks for looking at this so quickly! I'll look/test more closely later tonight (after the DayJob), but from some quick testing I found at least one oddity: 1. Open the attached in Epiphany. 2. In Accerciser, expand the accessible associated with the list and highlight the first list item. 3. acc.getIndexInParent() 4. Choose the next list item. 5. acc.getIndexInParent() Rinse and repeat for each immediate child of the list. Here's what I get: 1st child: 0 2nd child: 0 <-- should be 1 3rd child: 1 <-- should be 2 4th child: 2 <-- should be 3 5th child: 4 6th child: 5 7th child: 6 And, yes, given that there are 5 list items rather than 7, it would seem I've found another bug. :-) I'll file it shortly. But given the rendering of the list as having 7 children, I'd expect those children to have the indexes indicated above.
Joanmarie Diggs
Comment 3
2009-07-08 12:07:45 PDT
(In reply to
comment #1
)
> Created an attachment (id=32456) [details] > getindexinparent.patch > > This implements the function, but the particular testcase you mention does not > work because the object hierarchy is not quite right in Google's case (the Map > object has a dummy 'label' parent).
Actually, my test case does work with your patch. :-) Ignore the "label" graphic in Accerciser. The Role column says it's a link. If I highlight the accessible of ROLE_LINKN for "Maps" (not the accessible text object which is the child of that link), acc.getIndexInParent() returns 3 as expected. Yea! I did find another oddity on Google though. If you look at the hierarchy in Accerciser, you should see: document frame panel text link link link link link link link panel panel panel panel panel panel panel (I expanded the first panel just to be sure we're on the same page so to speak.) Looking strictly at the panels/immediate children of the document frame, try acc.getIndexInParent() I get: 1st child: 0 2nd child: 1 3rd child: 0 4th child: 0 5th child: 1 6th child: 2 7th child: 4 8th child: 5 So independent of the list rendering issue I reported in my previous comment, I think there may be something not quite right about your patch. Sorry!
Jan Alonzo
Comment 4
2009-07-09 15:30:04 PDT
Comment on
attachment 32456
[details]
getindexinparent.patch
> - // FIXME: This needs to be implemented. > - notImplemented(); > + AccessibilityObject* coreObject = core(object); > + AccessibilityObject* parent = coreObject->parentObject(); > + > + g_return_val_if_fail (parent, 0);
Just a nit: this needs to be in "if" since we're dealing with WebCore objects. I'll clear the review flag for now until we sort out Joanmarie's findings.
Xan Lopez
Comment 5
2009-07-11 12:17:04 PDT
(In reply to
comment #4
)
> (From update of
attachment 32456
[details]
) > > - // FIXME: This needs to be implemented. > > - notImplemented(); > > + AccessibilityObject* coreObject = core(object); > > + AccessibilityObject* parent = coreObject->parentObject(); > > + > > + g_return_val_if_fail (parent, 0); > > Just a nit: this needs to be in "if" since we're dealing with WebCore objects.
I don't understand what you mean here.
> > I'll clear the review flag for now until we sort out Joanmarie's findings.
Joannie, do you think it's better to land this as it is and fix things incrementally or would be this patch worse than no implementation?
Joanmarie Diggs
Comment 6
2009-07-11 16:22:15 PDT
(In reply to
comment #5
)
> Joannie, do you think it's better to land this as it is and fix things > incrementally or would be this patch worse than no implementation?
It's up to you. If you'd prefer to check in what you have, I'd be happy to open a new bug for the two issues I found. From my perspective, since I cannot rely upon the return value, I am not going to implement anything in Orca that requires it.
Joanmarie Diggs
Comment 7
2009-10-26 12:41:28 PDT
After staring at/working on hierarchy issues all this weekend, it dawned on me what the problem might be:
> + AccessibilityObject* coreObject = core(object); > + AccessibilityObject* parent = coreObject->parentObject();
The parentObject() is from the dom. Instead use parentObjectUnignored(). I made the change and tested quite a bit. It works as expected.
Xan Lopez
Comment 8
2009-10-26 13:02:50 PDT
Created
attachment 41884
[details]
getindexinparent.patch New version using parentObjectUnignored as proposed by Joannie.
Gustavo Noronha (kov)
Comment 9
2009-10-26 15:05:14 PDT
Comment on
attachment 41884
[details]
getindexinparent.patch
> + g_return_val_if_fail (parent, 0);
Space before ( here.
> + AccessibilityObject::AccessibilityChildrenVector children = parent->children(); > + unsigned count = children.size(); > + for (unsigned k = 0; k < count; ++k) { > + if (children[k] == coreObject) > + return k; > + }
k? i sounds more idiomatic ;D also, why not use children.size() directly instead of creating a variable? Except for the nits, I see no obvious problem, and trust Joanmarie's judgement.
Joanmarie Diggs
Comment 10
2009-10-26 15:28:17 PDT
Just noticed this: ** (GtkLauncher:3481): CRITICAL **: gint webkit_accessible_get_index_in_parent(AtkObject*): assertion `parent' failed Web views claim to be orphans. Not a big deal, but we should address it. In separate patch of course. :-P (I hacked around this same issue in webkit_accessible_get_parent.)
Xan Lopez
Comment 11
2009-10-27 01:28:40 PDT
(In reply to
comment #9
)
> (From update of
attachment 41884
[details]
) > > > + g_return_val_if_fail (parent, 0); > > Space before ( here.
Right.
> > > + AccessibilityObject::AccessibilityChildrenVector children = parent->children(); > > + unsigned count = children.size(); > > + for (unsigned k = 0; k < count; ++k) { > > + if (children[k] == coreObject) > > + return k; > > + } > > k? i sounds more idiomatic ;D also, why not use children.size() directly > instead of creating a variable?
Just a micro-optimization to not evaluate children.size() multiple times.
> > Except for the nits, I see no obvious problem, and trust Joanmarie's judgement.
Thanks!
Xan Lopez
Comment 12
2009-10-27 01:34:37 PDT
(In reply to
comment #10
)
> Just noticed this: > > ** (GtkLauncher:3481): CRITICAL **: gint > webkit_accessible_get_index_in_parent(AtkObject*): assertion `parent' failed > > Web views claim to be orphans. Not a big deal, but we should address it. In > separate patch of course. :-P > > (I hacked around this same issue in webkit_accessible_get_parent.)
Hrm. I also see get_parent is using parentObject instead of parentObjectUnignored? ;)
Xan Lopez
Comment 13
2009-10-27 02:26:13 PDT
Comment on
attachment 41884
[details]
getindexinparent.patch Landed in
r50134
, leaving open to deal with the critical warning stuff.
Joanmarie Diggs
Comment 14
2009-10-27 06:29:13 PDT
(In reply to
comment #12
)
> (In reply to
comment #10
) > > Just noticed this: > > > > ** (GtkLauncher:3481): CRITICAL **: gint > > webkit_accessible_get_index_in_parent(AtkObject*): assertion `parent' failed > > > > Web views claim to be orphans. Not a big deal, but we should address it. In > > separate patch of course. :-P > > > > (I hacked around this same issue in webkit_accessible_get_parent.) > > Hrm. I also see get_parent is using parentObject instead of > parentObjectUnignored? ;)
Heh. Good point. Prior to this weekend's excursion into Hierarchy Hell, I didn't fully get what the unignored bit was all about.
Bug 30817
opened to evaluate that situation (and possibly remove the hack of calling the parent class).
Joanmarie Diggs
Comment 15
2009-11-02 22:04:00 PST
I issue per bug. Spun off
bug 31044
for the assertion. Closing this one.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug