Closed Bug 340670 Opened 19 years ago Closed 18 years ago

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

Categories

(Core :: Disability Access APIs, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: aaronlev, Assigned: gaomingcn)

References

(Blocks 1 open bug)

Details

(Keywords: access)

Attachments

(3 files, 4 obsolete files)

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"
Assignee: aaronleventhal → gaomingcn
Blocks: textattra11y
No longer blocks: newatk
I am trying to locate the code. in layout/ or content/ directory? Still investigating.
Attached patch Testcase (deleted) — Splinter Review
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.
Status: NEW → ASSIGNED
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.
Depends on: 346730, 346833
Attached patch patch traft. (obsolete) (deleted) — Splinter Review
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.
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.
Attached patch patch traft v2 (obsolete) (deleted) — Splinter Review
Add GetAttributeSet() to call GetAttributes() and use it in getRunAttributesCB().
Attachment #236714 - Attachment is obsolete: true
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.
Attached patch patch for nsHyperTextAccessible.cpp (obsolete) (deleted) — Splinter Review
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.
Attached patch patch v3 (obsolete) (deleted) — Splinter Review
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
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?
(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.
Attachment #237780 - Flags: review?(ginn.chen)
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)
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 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-
Attached patch patch v4 (deleted) — Splinter Review
Addressing Ginn and Aaron's comments. nsHTMLListBulletAccessible::GetAttributes() was deleted. That works.
Attachment #237780 - Attachment is obsolete: true
Attachment #238000 - Flags: review?(aaronleventhal)
Attachment #238000 - Flags: review?(aaronleventhal) → review+
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: