Last Comment Bug 343150 - Use AtkHyperlinkImpl
: Use AtkHyperlinkImpl
Status: RESOLVED FIXED
testcase-needed
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Nian Liu(n/a in a long time)
:
Mentors:
Depends on:
Blocks: newatk
  Show dependency treegraph
 
Reported: 2006-06-29 10:48 PDT by Aaron Leventhal
Modified: 2006-09-04 13:33 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (3.40 KB, patch)
2006-07-21 00:20 PDT, Nian Liu(n/a in a long time)
ginn.chen: review-
Details | Diff | Splinter Review
.h file (2.14 KB, patch)
2006-07-21 00:24 PDT, Nian Liu(n/a in a long time)
no flags Details | Diff | Splinter Review
.cpp file (2.52 KB, patch)
2006-07-21 00:25 PDT, Nian Liu(n/a in a long time)
ginn.chen: review-
Details | Diff | Splinter Review
patchv2 (3.51 KB, patch)
2006-07-23 23:39 PDT, Nian Liu(n/a in a long time)
ginn.chen: review-
Details | Diff | Splinter Review
patch (5.97 KB, patch)
2006-07-24 00:25 PDT, Nian Liu(n/a in a long time)
no flags Details | Diff | Splinter Review
patch (4.71 KB, patch)
2006-08-27 22:53 PDT, Nian Liu(n/a in a long time)
ginn.chen: review+
Details | Diff | Splinter Review
.h file (2.14 KB, patch)
2006-08-27 22:54 PDT, Nian Liu(n/a in a long time)
ginn.chen: review+
Details | Diff | Splinter Review
.cpp file (2.50 KB, patch)
2006-08-27 22:55 PDT, Nian Liu(n/a in a long time)
ginn.chen: review+
Details | Diff | Splinter Review
patch to remove compile error (203 bytes, patch)
2006-08-28 03:40 PDT, Nian Liu(n/a in a long time)
no flags Details | Diff | Splinter Review
patch to remove compile error (6.96 KB, patch)
2006-08-28 03:42 PDT, Nian Liu(n/a in a long time)
no flags Details | Diff | Splinter Review
patch to remove compile error v2(variable name convension modify) (6.96 KB, patch)
2006-08-28 19:49 PDT, Nian Liu(n/a in a long time)
no flags Details | Diff | Splinter Review
patch to remove compile error v2 (11.16 KB, patch)
2006-08-30 01:02 PDT, Ginn Chen
nian.liu: review+
Details | Diff | Splinter Review

Description Aaron Leventhal 2006-06-29 10:48:37 PDT
This is the Firefox portion of this Gnome bug:
http://bugzilla.gnome.org/show_bug.cgi?id=344284

There is a new interfaces to help us deal with the fact that links are not interfaces in ATK, but objects. In Firefox we just treat it like another interface.

Take a look at this include file:
#include <atk/atkhyperlinkimpl.h>
Comment 1 Nian Liu(n/a in a long time) 2006-07-21 00:20:57 PDT
Created attachment 230092 [details] [diff] [review]
patch

patch without new added files
Comment 2 Nian Liu(n/a in a long time) 2006-07-21 00:24:51 PDT
Created attachment 230093 [details] [diff] [review]
.h file
Comment 3 Nian Liu(n/a in a long time) 2006-07-21 00:25:27 PDT
Created attachment 230094 [details] [diff] [review]
.cpp file
Comment 4 Aaron Leventhal 2006-07-21 05:22:41 PDT
Comment on attachment 230094 [details] [diff] [review]
.cpp file

I'm traveling and won't be available to review for a few days. Anyway, Ginn understands that ATK/AT-SPI architecture better than I.
Comment 5 Ginn Chen 2006-07-23 23:01:29 PDT
Comment on attachment 230092 [details] [diff] [review]
patch

+    if (accessInterfaceHyperlink) {
+       interfacesBits |= 1 << MAI_INTERFACE_HYPERLINK;
+    }
     //nsIAccessibleTable

Nit: add a blank line after }

   nsMaiInterfaceTable.cpp \
+  nsMaiInterfaceHyperlinkImpl.cpp \
   $(NULL)
we need ATK version check here.
And we need to make sure it can work well with low version ATK.
Comment 6 Ginn Chen 2006-07-23 23:14:07 PDT
Comment on attachment 230094 [details] [diff] [review]
.cpp file

    nsresult rv;

not used.

Neo, do you have a method or python script to verify the patch?
Comment 7 Nian Liu(n/a in a long time) 2006-07-23 23:39:52 PDT
Created attachment 230398 [details] [diff] [review]
patchv2

addressing ginn's comment
Comment 8 Nian Liu(n/a in a long time) 2006-07-23 23:42:52 PDT
> Neo, do you have a method or python script to verify the patch?
> 

nope:(

it's not called from firefox side. add bill.haneman@sun.com to see if he can make one
Comment 9 Ginn Chen 2006-07-23 23:49:10 PDT
Comment on attachment 230398 [details] [diff] [review]
patchv2

1) it should be ATK 1.12.1
2) it won't compile with ATK below 1.12.1
because you uses ATK_TYPE_HYPERLINK_IMPL in nsAccessibleWrap.cpp
Comment 10 Nian Liu(n/a in a long time) 2006-07-24 00:25:18 PDT
Created attachment 230402 [details] [diff] [review]
patch

oops, strong coffee needed
Comment 11 Aaron Leventhal 2006-08-02 13:55:48 PDT
Where are we at with this patch? It's probably the most important thing we need right now.
Comment 12 Ginn Chen 2006-08-02 23:46:27 PDT
(In reply to comment #11)
> Where are we at with this patch? It's probably the most important thing we need
> right now.
> 

I would like to have a unit test to make sure it really works before checking in.
Comment 13 Aaron Leventhal 2006-08-14 13:33:31 PDT
Ginn, would it help with testing to get this in? At least that way it won't bitrot. We can always fix it or totally remove it if it ends up being wrong in tests. It is trunk, and as long as we don't break anything too bad I think we should check it in if the code looks right.
Comment 14 Ginn Chen 2006-08-14 20:09:39 PDT
at-spi doesn't have AtkHyperlinkImpl interface yet.
So these code can do nothing now.
Comment 15 Aaron Leventhal 2006-08-24 06:29:44 PDT
Now that we're building with recent ATK headers, we should have an easier time testing this, correct? It seems like we should get this in soon, because it is blocking more testing by screen reader developers.

The AT-SPI Hyperlink interface is returning incorrect info for startIndex, endIndex and crashing for getURI. We think this will probably fix that.
Comment 16 Nian Liu(n/a in a long time) 2006-08-27 22:53:33 PDT
Created attachment 235702 [details] [diff] [review]
patch

verified with at-poke at-spi atk link
Comment 17 Nian Liu(n/a in a long time) 2006-08-27 22:54:47 PDT
Created attachment 235703 [details] [diff] [review]
.h file
Comment 18 Nian Liu(n/a in a long time) 2006-08-27 22:55:55 PDT
Created attachment 235705 [details] [diff] [review]
.cpp file
Comment 19 Ginn Chen 2006-08-27 23:49:44 PDT
patch backed out

It could not link on tinderbox machine.

undefined reference to `atk_hyperlink_impl_get_type'
Comment 20 Nian Liu(n/a in a long time) 2006-08-28 03:40:56 PDT
Created attachment 235729 [details] [diff] [review]
patch to remove compile error
Comment 21 Nian Liu(n/a in a long time) 2006-08-28 03:42:40 PDT
Created attachment 235730 [details] [diff] [review]
patch to remove compile error
Comment 22 Nian Liu(n/a in a long time) 2006-08-28 19:49:51 PDT
Created attachment 235853 [details] [diff] [review]
patch to remove compile error v2(variable name convension modify)
Comment 23 Ginn Chen 2006-08-30 01:02:29 PDT
Created attachment 236042 [details] [diff] [review]
patch to remove compile error v2

I changed a little bit.
Comment 24 Nian Liu(n/a in a long time) 2006-08-30 01:23:00 PDT
Comment on attachment 236042 [details] [diff] [review]
patch to remove compile error v2

fine for me
Comment 25 Nian Liu(n/a in a long time) 2006-09-04 01:12:38 PDT
(In reply to comment #15)
> Now that we're building with recent ATK headers, we should have an easier time
> testing this, correct? It seems like we should get this in soon, because it is
> blocking more testing by screen reader developers.
> 
> The AT-SPI Hyperlink interface is returning incorrect info for startIndex,
> endIndex and crashing for getURI. We think this will probably fix that.
> 

Aaron, this patch does no help on above issue. 
Comment 26 Aaron Leventhal 2006-09-04 13:33:21 PDT
Okay, file a bug. I think Ginn Chen will need to look at it, unless you think you can debug it.

Note You need to log in before you can comment on or make changes to this bug.