Closed Bug 151114 Opened 22 years ago Closed 22 years ago

Support nsIAccessibleHyperLink in MAI (Mozilla Atk Implementation)

Categories

(Core :: Disability Access APIs, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: yinbolian, Assigned: yinbolian)

References

Details

(Keywords: access)

Attachments

(1 file, 3 obsolete files)

 
Blocks: 145863
Status: NEW → ASSIGNED
Keywords: access
QA Contact: dsirnapalli → mindy.liu
Attached patch patch_v1 (obsolete) — Splinter Review
nsIAccessibleHyperLink is different with other nsIAccessibleXXX interfaces in
that is a auxiliary interface for nsIAccessibleHyperText. So a
nsIAccessibleHyperLink instance is not necessarily a nsIAccessible. In face, in
atk atkhyperlink is a GObject while atkhypertext, atkaction, atktext, etc. are
GTypeInterface's.
Attachment #87497 - Attachment is obsolete: true
Whiteboard: need r=
-> gilbert
Comment on attachment 88434 [details] [diff] [review]
patch_v2 (move callbacks into C namespace)

>+
>+static gpointer parent_class = NULL;
>+
Is there any possibility to change the name of "parent_class" to
"super_class_AtkHyperlink"?


>+
>+MaiHyperlink::MaiHyperlink(nsIAccessibleHyperLink *aAcc)
>+{
>+    mHyperlink = aAcc;
>+    mMaiAtkHyperlink = NULL;
>+    mURI = (char*)NULL;
>+}
Would it be better if  aAcc is changed to aAccHyperLink?

>+    MaiHyperlink::Initialize(mMaiAtkHyperlink, this);
>+
>+    return (AtkHyperlink*)mMaiAtkHyperlink;
If the caller or AtkHyperlink itself can manage the life cycle itself, it is
okay here.  
>+
>+    MaiObject *maiObj = maiHyperlink->GetObject(aLinkIndex);
>+    if (!maiObj)
>+        return NULL;
>+
>+    //no need to add ref it, because it is "get" not "ref" ???
>+    return maiObj->GetAtkObject();
>+}
It must be sure this maiObj will be released somewhere when it should be .
Other parts of this patch  is  okay from my view.
Thanks, Gilbert. 
The lifecycle of mai object and mai Hyperlink will be controled by maiCache
(later), I will be careful on that, and you are welcome to check it later.
I see your point of variable names, but their usage are all restricted in narrow
scope, so their meaning is clear in the context :). 
Attached patch patch_v3 (obsolete) — Splinter Review
comments in bug 151133 hold here
Attachment #88434 - Attachment is obsolete: true
Attached patch patch v4Splinter Review
make return statement simpler
Attachment #89221 - Attachment is obsolete: true
Comment on attachment 89244 [details] [diff] [review]
patch v4

r=aaronl
Attachment #89244 - Flags: review+
Depends on: 150603
Comment on attachment 89244 [details] [diff] [review]
patch v4

sr=jst
Attachment #89244 - Flags: superreview+
Checked in! Thanks for everyone!
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Whiteboard: need r=
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: