If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Support nsIAccessibleHyperLink in MAI (Mozilla Atk Implementation)

RESOLVED FIXED

Status

()

Core
Disability Access APIs
RESOLVED FIXED
16 years ago
15 years ago

People

(Reporter: Bolian Yin, Assigned: Bolian Yin)

Tracking

({access})

Trunk
x86
Linux
access
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

16 years ago
 
(Assignee)

Updated

16 years ago
Blocks: 145863
Status: NEW → ASSIGNED
Keywords: access
(Assignee)

Updated

16 years ago
QA Contact: dsirnapalli → mindy.liu
(Assignee)

Comment 1

16 years ago
Created attachment 87497 [details] [diff] [review]
patch_v1

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.
(Assignee)

Comment 2

16 years ago
Created attachment 88434 [details] [diff] [review]
patch_v2 (move callbacks into C namespace)
Attachment #87497 - Attachment is obsolete: true
(Assignee)

Updated

16 years ago
Whiteboard: need r=

Comment 3

16 years ago
-> gilbert

Comment 4

16 years ago
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 .

Comment 5

16 years ago
Other parts of this patch  is  okay from my view.
(Assignee)

Comment 6

16 years ago
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 :). 
(Assignee)

Comment 7

16 years ago
Created attachment 89221 [details] [diff] [review]
patch_v3

comments in bug 151133 hold here
Attachment #88434 - Attachment is obsolete: true
(Assignee)

Comment 8

16 years ago
Created attachment 89244 [details] [diff] [review]
patch v4

make return statement simpler
Attachment #89221 - Attachment is obsolete: true

Comment 9

16 years ago
Comment on attachment 89244 [details] [diff] [review]
patch v4

r=aaronl
Attachment #89244 - Flags: review+

Updated

16 years ago
Depends on: 150603
Comment on attachment 89244 [details] [diff] [review]
patch v4

sr=jst
Attachment #89244 - Flags: superreview+
(Assignee)

Comment 11

15 years ago
Checked in! Thanks for everyone!
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
Whiteboard: need r=
You need to log in before you can comment on or make changes to this bug.