The default bug view has changed. See this FAQ.

New ATK: Expose static text from layout with text attribute "static=true"

RESOLVED FIXED

Status

()

Core
Disability Access APIs
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Aaron Leventhal, Assigned: Mike Gao)

Tracking

(Blocks: 1 bug, {access})

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

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 4 obsolete attachments)

(Reporter)

Description

11 years ago
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"
(Reporter)

Updated

11 years ago
Assignee: aaronleventhal → gaomingcn
Blocks: 340809
No longer blocks: 333492
(Assignee)

Comment 1

11 years ago
I am trying to locate the code. in layout/ or content/ directory?  Still investigating.
(Reporter)

Comment 2

11 years ago
Created attachment 229077 [details] [diff] [review]
Testcase
(Reporter)

Comment 3

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

Updated

11 years ago
Status: NEW → ASSIGNED
(Reporter)

Comment 4

11 years ago
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.
(Reporter)

Updated

11 years ago
Depends on: 346730, 346833
(Assignee)

Comment 5

11 years ago
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.
(Reporter)

Comment 6

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

Comment 7

11 years ago
Created attachment 236984 [details] [diff] [review]
patch traft v2

Add GetAttributeSet() to call GetAttributes() and use it in getRunAttributesCB().
Attachment #236714 - Attachment is obsolete: true
(Reporter)

Comment 8

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

Comment 9

11 years ago
Created attachment 237264 [details] [diff] [review]
patch for nsHyperTextAccessible.cpp
(Reporter)

Comment 10

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

Comment 11

11 years ago
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.
Attachment #236984 - Attachment is obsolete: true
Attachment #237264 - Attachment is obsolete: true
(Assignee)

Comment 12

11 years ago
Created attachment 237781 [details]
screen shot of not exposing static but tag name
(Reporter)

Comment 13

11 years ago
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?
(Assignee)

Comment 14

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

Updated

11 years ago
Attachment #237780 - Flags: review?(ginn.chen)

Comment 15

11 years ago
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.
Attachment #237780 - Flags: review?(ginn.chen) → review?(aaronleventhal)
(Assignee)

Comment 16

11 years ago
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"?
(Reporter)

Comment 17

11 years ago
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
Attachment #237780 - Flags: review?(aaronleventhal) → review-
(Assignee)

Comment 18

11 years ago
Created attachment 238000 [details] [diff] [review]
patch v4

Addressing Ginn and Aaron's comments. nsHTMLListBulletAccessible::GetAttributes() was deleted. That works.
Attachment #237780 - Attachment is obsolete: true
Attachment #238000 - Flags: review?(aaronleventhal)
(Reporter)

Updated

11 years ago
Attachment #238000 - Flags: review?(aaronleventhal) → review+
(Reporter)

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.