Last Comment Bug 689540 - Expose IA2 margin- object attributes
: Expose IA2 margin- object attributes
Status: RESOLVED FIXED
[good first bug][mentor=surkov.alexan...
: dev-doc-complete
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: x86 Windows 7
: P3 normal (vote)
: mozilla13
Assigned To: Mark Capella [:capella]
:
Mentors:
http://nightly.ckeditor.com/7291/_sam...
Depends on:
Blocks: jaws
  Show dependency treegraph
 
Reported: 2011-09-27 05:37 PDT by Frank DiPalermo
Modified: 2012-04-30 02:58 PDT (History)
7 users (show)
mzehe: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
First bugfix attempt ... DIFF file (5.92 KB, patch)
2012-02-05 11:16 PST, Mark Capella [:capella]
surkov.alexander: review+
Details | Diff | Splinter Review
Fixed (5.84 KB, patch)
2012-02-05 22:09 PST, Mark Capella [:capella]
surkov.alexander: review+
Details | Diff | Splinter Review

Description Frank DiPalermo 2011-09-27 05:37:46 PDT
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.
Comment 1 Marco Zehe (:MarcoZ) 2011-09-27 05:53:17 PDT
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.
Comment 2 Frank DiPalermo 2011-09-27 06:01:46 PDT
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.
Comment 3 Marco Zehe (:MarcoZ) 2011-09-27 06:05:44 PDT
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.
Comment 4 alexander :surkov 2011-09-27 06:08:28 PDT
Frank, it would be helpful to know how they set up the indentation, for example, code snippet.
Comment 5 James Teh [:Jamie] 2011-09-27 15:24:42 PDT
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.
Comment 6 alexander :surkov 2011-09-28 17:55:08 PDT
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
Comment 8 Mark Capella [:capella] 2012-02-05 11:16:07 PST
Created attachment 594569 [details] [diff] [review]
First bugfix attempt ... DIFF file

   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.
Comment 9 alexander :surkov 2012-02-05 21:42:22 PST
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
Comment 10 Mark Capella [:capella] 2012-02-05 22:09:44 PST
Created attachment 594637 [details] [diff] [review]
Fixed

Fixed
Comment 11 David Bolter [:davidb] 2012-02-06 07:03:00 PST
Thanks Mark!
Comment 12 Marco Zehe (:MarcoZ) 2012-02-06 09:30:04 PST
Landed this on Mark's behalf on mozilla-inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/50c00def777a

Thank you for the patch, Mark!
Comment 13 Mark Capella [:capella] 2012-02-06 09:34:37 PST
   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.
Comment 14 Marco Zehe (:MarcoZ) 2012-02-06 10:02:37 PST
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.
Comment 15 Ed Morley [:emorley] 2012-02-07 12:17:36 PST
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
Comment 16 alexander :surkov 2012-03-12 14:42:21 PDT
https://developer.mozilla.org/en/Accessibility/AT-APIs/Gecko/Attrs should be updated
Comment 17 Eric Shepherd [:sheppy] 2012-04-29 11:46:12 PDT
Documentation updated:

https://developer.mozilla.org/en/Accessibility/AT-APIs/Gecko/Attrs#Hypertext_attributes

Mentioned on Firefox 13 for developers.
Comment 18 alexander :surkov 2012-04-30 02:58:38 PDT
Thank you!

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