Last Comment Bug 340670 - New ATK: Expose static text from layout with text attribute "static=true"
: New ATK: Expose static text from layout with text attribute "static=true"
Status: RESOLVED FIXED
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Mike Gao
:
Mentors:
Depends on: 346730 346833
Blocks: textattra11y
  Show dependency treegraph
 
Reported: 2006-06-07 06:37 PDT by Aaron Leventhal
Modified: 2006-09-12 10:06 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase (935 bytes, patch)
2006-07-13 05:43 PDT, Aaron Leventhal
no flags Details | Diff | Splinter Review
patch traft. (1.57 KB, patch)
2006-09-04 09:42 PDT, Mike Gao
no flags Details | Diff | Splinter Review
patch traft v2 (2.94 KB, patch)
2006-09-06 11:58 PDT, Mike Gao
no flags Details | Diff | Splinter Review
patch for nsHyperTextAccessible.cpp (1.49 KB, patch)
2006-09-07 18:51 PDT, Mike Gao
no flags Details | Diff | Splinter Review
patch v3 (5.89 KB, patch)
2006-09-11 11:01 PDT, Mike Gao
aaronlev: review-
Details | Diff | Splinter Review
screen shot of not exposing static but tag name (13.60 KB, image/jpeg)
2006-09-11 11:03 PDT, Mike Gao
no flags Details
patch v4 (6.32 KB, patch)
2006-09-12 09:45 PDT, Mike Gao
aaronlev: review+
Details | Diff | Splinter Review

Description Aaron Leventhal 2006-06-07 06:37:27 PDT
Occasionally the layout engine inserts text for us, such as numbers or letters in ordered lists, or :before and :after styled text. The readonly nature of this text should be indicated with a text attribute "static=true"
Comment 1 Mike Gao 2006-07-13 04:03:25 PDT
I am trying to locate the code. in layout/ or content/ directory?  Still investigating.
Comment 2 Aaron Leventhal 2006-07-13 05:43:08 PDT
Created attachment 229077 [details] [diff] [review]
Testcase
Comment 3 Aaron Leventhal 2006-07-13 05:51:43 PDT
You don't need anything in layout or content. You can already tell that something is static text because it uses ROLE_STATICTEXT.

Let's say you have an nsHyperTextAccessible for a heading, and the heading has before text of "Section: "

<h1>Lawnmowing in the rain</h1>
will be rendered as 
Section: Gardening

In the a11y hierarchy you will see

* Parent ROLE_HEADING (nsHyperTextAccessible), text="Section: Gardening"
   * Child ROLE_STATIC_TEXT, name="Section: "
   * Child ROLE_TEXT_LEAF, name="Gardening"

So the parent ROLE_HEADING needs to have a text attribute run from 0 to 9 of "static=true". There is a method called nsHyperTextAccessible::GetAttributeRange() which would return the ROLE_STATIC_TEXT_CHILD, and 0-9 for the offsets.

Then there must be something in  atk/nsAccessibleWrap which uses that info and exposes the object attributes for that accessible and that range.
Comment 4 Aaron Leventhal 2006-07-20 07:43:58 PDT
Also, in nsHyperTextAccessible we're not treating statictext right. We need to have a case for ROLE_STATICTEXT where we append the text for the statictext object in GetText() and deal with it wherever we deal with ROLE_TEXT_LEAF.
Comment 5 Mike Gao 2006-09-04 09:42:35 PDT
Created attachment 236714 [details] [diff] [review]
patch traft.

With https://bugzilla.mozilla.org/attachment.cgi?id=234809 applied. When I test, :before and :after text is exposed as a whole with the content. I think we need to separate them and expose static attribute for :before and :after text only.
Comment 6 Aaron Leventhal 2006-09-05 17:47:39 PDT
What I want you to do is use
accessibleWithAttrs->GetAttributes() to find out what text attributes to expose.

Then, on any object that is a static text, I want you to expose static=true via ::GetAttributes()

This will be more extensible when we want to add more text attributes, I don't want the logic hard coded into each platform API. Remember that eventually we will be exposing all of this information on Windows and Mac as well.
Comment 7 Mike Gao 2006-09-06 11:58:45 PDT
Created attachment 236984 [details] [diff] [review]
patch traft v2

Add GetAttributeSet() to call GetAttributes() and use it in getRunAttributesCB().
Comment 8 Aaron Leventhal 2006-09-06 12:45:52 PDT
Comment on attachment 236984 [details] [diff] [review]
patch traft v2

I'm expecting to see a change to nsListBulletAccessible and nsHTMLTextAccessible so that they now implement ::GetAttributes(). In the case of nsHTMLTextAccessible::GetAttributes() it will have to be smart about when it says static=true.
Comment 9 Mike Gao 2006-09-07 18:51:47 PDT
Created attachment 237264 [details] [diff] [review]
patch for nsHyperTextAccessible.cpp
Comment 10 Aaron Leventhal 2006-09-07 18:55:20 PDT
nsHTMLListBulletACcessible::GetAttributes() should just return static=true always
and nsHTMLTextACcessible::GetRole() should return ROLE_STATICTEXT when it's static
and nsHTMLTextAccessible::GetAttributes() should call GetRole() and return static=true if it's ROLE_STATICTEXT

Then in your ATK method, when you do:
nsresult rv = accText->GetAttributeRange(aOffset, startOffset, &endOffset, getter_AddRefs(accessibleWithAttrs));
You will need to use accessibleWithAttrs->GetAttributes() and use that to expose the text attributes for the range.
Comment 11 Mike Gao 2006-09-11 11:01:40 PDT
Created attachment 237780 [details] [diff] [review]
patch v3

static=true is exposed for bullet and static text. This is tested with at-poke.

But when test with the tescase in this bug, for line #5, "tag:A" is exposed instead of "static:true". A picture will be posted.
Comment 12 Mike Gao 2006-09-11 11:03:43 PDT
Created attachment 237781 [details]
screen shot of not exposing static but tag name
Comment 13 Aaron Leventhal 2006-09-11 11:10:21 PDT
That seems to make sense in that the caret offset is at 0, which is where the embedded offset char is.

What if you move the caret to offset 5 or something?
Comment 14 Mike Gao 2006-09-11 23:24:18 PDT
(In reply to comment #13)
> That seems to make sense in that the caret offset is at 0, which is where the
> embedded offset char is.
> 
> What if you move the caret to offset 5 or something?
> 

yes, it changed when I move the caret offset. I am requesting review from Ginn then.
Comment 15 Ginn Chen 2006-09-12 00:30:03 PDT
Comment on attachment 237780 [details] [diff] [review]
patch v3

I'm not sure if we want to implement nsHTMLTextAccessible::GetRole


Can we remove this XXX?
     // XXX Turn accessibleWithAttrs into AtkAttributeSet by getting CSS for it
     // Look at  nsAccessNodeWrap::get_computedStyle() for MSAA implementation


aIAccessible doesn't look like a good name to me.
Comment 16 Mike Gao 2006-09-12 03:04:05 PDT
Thanks for your comments. Below is my reply.

(In reply to comment #15)
> (From update of attachment 237780 [details] [diff] [review] [edit])
> I'm not sure if we want to implement nsHTMLTextAccessible::GetRole
> 
nsHTMLTextAccessible::GetAttributes() will use it. We cann't get ROLE_STATICTEXT without it.

> 
> Can we remove this XXX?
>      // XXX Turn accessibleWithAttrs into AtkAttributeSet by getting CSS for it
>      // Look at  nsAccessNodeWrap::get_computedStyle() for MSAA implementation
> 

Sure.

> 
> aIAccessible doesn't look like a good name to me.
> 
use "nsAccessible* aAccessible" instead of "nsIAccessible* aIAccessible"?
Comment 17 Aaron Leventhal 2006-09-12 07:10:22 PDT
Comment on attachment 237780 [details] [diff] [review]
patch v3

+NS_IMETHODIMP nsHTMLTextAccessible::GetRole(PRUint32 *aRole)
1) Do a null check on the frame here.
2) For non statictext case, do a |return nsTextAccessible::GetRole(aRole);
You may need to reorganize things to avoid an extra else, once you do that.

I have a new thought. Since sHTMLListBulletAccessible inherits from nsHTMLTextAccessible, and uses ROLE_STATIC_TEXT, you should be able to remove nsHTMLListBulletAccessible::GetAttributes() and it will still work for list bullets! :) Does that work?

Change name of aIAccessible to aAccessible
Comment 18 Mike Gao 2006-09-12 09:45:48 PDT
Created attachment 238000 [details] [diff] [review]
patch v4

Addressing Ginn and Aaron's comments. nsHTMLListBulletAccessible::GetAttributes() was deleted. That works.

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