Closed Bug 689540 Opened 13 years ago Closed 12 years ago

Expose IA2 margin- object attributes

Categories

(Core :: Disability Access APIs, defect, P3)

x86
Windows 7
defect

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: dipalerm, Assigned: capella)

References

(Blocks 1 open bug, )

Details

(Keywords: dev-doc-complete, Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=c++])

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:6.0.2) Gecko/20100101 Firefox/6.0.2
Build ID: 20110902133214

Steps to reproduce:

Created lines in CK Editor with various indentions.  Here is the link to CK Editor:
http://nightly.ckeditor.com/7291/_samples/replacebyclass.html


Actual results:

Looked at the exposed MSAA attributes and found that all lines are exposed with a 0 pixel indent.


Expected results:

Indented lines should have correct pixel indent levels.
Blocks: jaws
Priority: -- → P3
I believe what we should do is not show an indention by a number of pixels, but indicate, similar to read-only html documents, the indentation level/blockquote level, much like we do when citing messages in Thunderbird. I believe the object attributes in IA2 already expose something to that effect, but I'd have to look up the spec to be certain. In any case, CK Editor needs to tell us about the indentation levels through its code, or we won't have a chance to see them at all.
Component: General → Disability Access APIs
Product: Firefox → Core
QA Contact: general → accessibility-apis
Version: 6 Branch → Trunk
From Frank on 9/27/2011:
Marco, I queried the CK Editor folks on this and they responded that they were exposing a 40 pixel incrment for each indention, but we observed that Firefox always says that the indent level is 0 pixels.
The question is *how* they expose that indentation. If it's through pure CSS, it is very likely we are not picking that up, because there is no standard this information is to be translated from CSS to a11y APIs yet. Other editors expose stuff like this through blockquote elements that are styled in accordance with the wanted indentation level. This way we would pick it up automatically.  That would not give you exact pixels, but an idea about the indentation anyway.
Frank, it would be helpful to know how they set up the indentation, for example, code snippet.
The text-indent object attribute seems to correspond to the text-indent CSS property, which specifies "the indentation of the first line of text in a block container", not the indentation of the entire block. If I do:
<p style="text-indent: 5px">blah</p>
Firefox correctly exposes 5px in the text-indent object attribute.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Firefox does not expose line levels in CK Editor - Accessibility Issue → Expose IA2 margin- object attributes
All we need to do is fix nsAccessible::GetAttrbutesInternal (http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsAccessible.cpp#1414) the same way how we expose other CSS based object attributes.

Keep in mind to add tests: attributes/test_obj_css.html
Whiteboard: [good first bug]
Whiteboard: [good first bug] → [good first bug][mentor=surkov.alexander@gmail.com]
Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com] → [good first bug][mentor=surkov.alexander@gmail.com][lang=c++]
Can someone review this? If it's going in the right direction, I'd like to be added as the "Assign to"...

   I noticed CK Editor uses margin-left (40 pixel increment as noted) not left-indent. I'm not sure how (Frank DiPalermo) looked at the exposed MSAA attributes, maybe this has something to do with it.

   Also, the IA2 spec indicates margin property <length> values be in mm but px works also, so that's how I left the test HTML.

   Finally, the request indicates exposing "margin" attributes, so I didn't address object attributes: line-height, line-spacing, and tab-stop.
Attachment #594569 - Attachment is patch: true
Assignee: nobody → markcapella
Comment on attachment 594569 [details] [diff] [review]
First bugfix attempt ... DIFF file

Review of attachment 594569 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with comment fixed

::: content/base/src/nsGkAtomList.h
@@ +1997,5 @@
>  GK_ATOM(textAlign, "text-align")
> +GK_ATOM(marginLeft, "margin-left")
> +GK_ATOM(marginRight, "margin-right")
> +GK_ATOM(marginTop, "margin-top")
> +GK_ATOM(marginBottom, "margin-bottom")

nsGkAtomsList.h keeps atoms in alphabetical order
Attachment #594569 - Flags: review+
Attached patch FixedSplinter Review
Fixed
Attachment #594637 - Flags: review?(surkov.alexander)
Attachment #594637 - Flags: review?(surkov.alexander) → review+
Attachment #594569 - Attachment is obsolete: true
Landed this on Mark's behalf on mozilla-inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/50c00def777a

Thank you for the patch, Mark!
Flags: in-testsuite+
   You're all very welcome :-P

   Let me know if there's anything I need to do to "complete" this ... if it's safely in your court now I'll be looking at Bug 672202 - Don't completely hide aria-valuenow exposure with aria-valuetext exposure.
Mark, no, go right ahead! If there's anything that we need to follow up with you, you'll see it appear on this bug.
Thanks Mark - this has now been merged to mozilla-central & will be present in tomorrow's Nightly :-)

https://hg.mozilla.org/mozilla-central/rev/50c00def777a
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
Thank you!
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: