| Summary: | [ATK] accessibility/title-ui-element-correctness.html fails | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||||
| Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, jcraig, jdiggs, k.czech, mario, mpakulavelrutka, samuel_white | ||||||
| Priority: | P2 | Keywords: | Gtk, LayoutTestFailure | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Zan Dobersek
2012-10-19 03:00:43 PDT
*** Bug 112019 has been marked as a duplicate of this bug. *** Currently only one test case fails:
PASS reparentNodeIntoContainer(document.getElementById('control5'), document.getElementById('label5')); axElement('control5').titleUIElement() != null is true
PASS axElement('control5').titleUIElement().isEqual(axElement('label5')) is true
PASS axElement('control6').titleUIElement().isEqual(axElement('label6b')) is true
-PASS newLabel6Element = createLabelWithIdAndForAttribute('label6a', 'control6'); document.body.insertBefore(newLabel6Element, document.body.firstChild); axElement('control6').titleUIElement().isEqual(axElement('label6a')) is true
+FAIL newLabel6Element = createLabelWithIdAndForAttribute('label6a', 'control6'); document.body.insertBefore(newLabel6Element, document.body.firstChild); axElement('control6').titleUIElement().isEqual(axElement('label6a')) should be true. Was false.
PASS document.body.removeChild(newLabel6Element); axElement('control6').titleUIElement().isEqual(axElement('label6b')) is true
PASS successfullyParsed is true
In this test case a new label for control6 element is added before the existing one, and it should be returned when calling titleUIElement.
When setting a new relation for an object which currently has one, atk_relation_set_add_relation_by_type adds a relation as next element of relation's target list, while in DRT/WTR implementation we take into account only the first target.
Created attachment 216163 [details]
proposed patch
This patch fixes the bug by removing current relations before adding a new one.
Comment on attachment 216163 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=216163&action=review > Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:194 > +static void removeAtkRelationFromRelationSetByType(AtkRelationSet* relationSet, AtkRelationType relationType) I think removeAtkRelationByType is enough for the name > Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:199 > + if (atk_relation_get_relation_type(relation) == ATK_RELATION_LABELLED_BY) { You should be comparing against your relationType parameter, not that hardcoded case > Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:201 > + break; You should not break here, as the same type of AtkRelation might exist between one base object and one or more target objects. So, if you are removing by type you should remove them all the relationships matching that type Comment on attachment 216163 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=216163&action=review >> Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:194 >> +static void removeAtkRelationFromRelationSetByType(AtkRelationSet* relationSet, AtkRelationType relationType) > > I think removeAtkRelationByType is enough for the name OK >> Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:199 >> + if (atk_relation_get_relation_type(relation) == ATK_RELATION_LABELLED_BY) { > > You should be comparing against your relationType parameter, not that hardcoded case my bad, I will fix it >> Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp:201 >> + break; > > You should not break here, as the same type of AtkRelation might exist between one base object and one or more target objects. So, if you are removing by type you should remove them all the relationships matching that type I am not sure about it. In atk_relation_set_add_relation_by_type, there is a check if relation of that type is present in relation set. If yes, a new target is added, but no new relation is added to relation set - so I assume there should be only one relation of given type in relation set. (In reply to comment #5) > [...] > > You should not break here, as the same type of AtkRelation might exist > > between one base object and one or more target objects. So, if you are > > removing by type you should remove them all the relationships matching that > > type > > I am not sure about it. In atk_relation_set_add_relation_by_type, there is a > check if relation of that type is present in relation set. If yes, a new > target is added, but no new relation is added to relation set - so I assume > there should be only one relation of given type in relation set. You are right. I've checked the code in atkrelationset.c and it's quite clear about that. Thanks for noticing Created attachment 216174 [details]
fixes after review
Comment on attachment 216174 [details] fixes after review Clearing flags on attachment: 216174 Committed r158757: <http://trac.webkit.org/changeset/158757> All reviewed patches have been landed. Closing bug. |