439566 - Include the css display property as an IAccessible2 object attribute

Status: VERIFIED FIXED

(Core :: Disability Access APIs, enhancement)

Description

[Michael Curran]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008061505 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008061505 Minefield/3.0pre

Currently Gecko provides a particular attribute via IAccessible2::attributes to hint at whether the node this IAccessible2 object is representing is a block element or not. This attribute is called 'formatting' and can have one possible value which  is 'block'.
However, as the IAccessible2 specification strives to have standard object attributes which have meaning across all applications, and have come from other well known standards (example CSS 2), it would be better if this attribute could be transformed in to something a bit more standard. Note that formatting:block only appears in Mozilla Gecko Accessibility.
Another problem with formatting:block is that sometimes this attribute does not appear on nodes that are clearly block nodes. An example is the body element (represented by an IA2 document object), which of course is a block element, but does not have formatting:block.
It is proposed that the CSS display property be exposed in its intireity instead of the formatting attribute, which would mean that hopefully we will see it everywhere, plus we will see more than just 'block'. We should be able to see 'inline' 'table' 'table-row' etc.
It is very useful for assistive technologies to know whether an object is block or inline when rendering a virtual representation of a document as then it can force line breaks in the most appropriate places. Currently a screen reader can use formatting:block if its there, but if its not, it must still guess whether it is block or inline, it can't simply assume its inline.
I'm not sure if it should just be called 'display' or  'css-display' or 'style-display'.
If formatting:block as a slightly different use to what I am aware of, I'm in favour of keeping formatting:block, but I still strongly suggest the addition of 'display'.

Reproducible: Always

Comment 1

[alexander :surkov (:asurkov)]

Marco, what do you think?

1) Keep "formatting" and add the attribute "display"
or
2) Remove "formatting" and add the attribute "display".

Does someone use the attribute "formatting" currently?

If the name "display" is ok for the new attribute?

Comment 2

[Marco Zehe (:MarcoZ)]

(In reply to comment #1)
> 1) Keep "formatting" and add the attribute "display"
> or
> 2) Remove "formatting" and add the attribute "display".

I'm leaning towards 2 to keep things clean. Since we only introduced this in our IA2 support, we should find out who besides the NVDA developers use this attribute currently, and alert them that we may change this to make sure we expose better info.

> Does someone use the attribute "formatting" currently?

I'll ask the commercial screen reader vendors.

> If the name "display" is ok for the new attribute?

What do you think about "css-display"? I think it should emphasize that this has been derived from the CSS styling and is nothing arbitrary we make up out of some other info.

Comment 3

[alexander :surkov (:asurkov)]

(In reply to comment #2)

> What do you think about "css-display"? I think it should emphasize that this
> has been derived from the CSS styling and is nothing arbitrary we make up out
> of some other info.
> 

I would suggest "display" because we have a lot of text attributes in the proposal (http://wiki.mozilla.org/Accessibility/TextAttributes) having exact css names (like "font-family", "background-color" and so on).

Comment 4

[Marco Zehe (:MarcoZ)]

Fine with me.

Comment 5

[Aaron Leventhal]

I put in formatting: block for one or more of the screen reader vendors.
My opinion is, keep the name "formatting". Otherwise different versions of Firefox will expose IA2 differently.

Even if the screen readers are just starting to update to use this, we don't want them to have to check for the attribute twice.

Comment 6

[alexander :surkov (:asurkov)]

(In reply to comment #5)
> I put in formatting: block for one or more of the screen reader vendors.
> My opinion is, keep the name "formatting". Otherwise different versions of
> Firefox will expose IA2 differently.
> 
> Even if the screen readers are just starting to update to use this, we don't
> want them to have to check for the attribute twice.
> 

But if we extend the possible values of the attribute then it will mean different versions of Firefox exposes IA2 differently too. No?

Comment 7

[Aaron Leventhal]

Extending values is okay. Any in-place logic that checks for specific values will still work.

Or do you have an example of something which may be done now which would no longer work?

Comment 8

[James Teh [:Jamie]]

(In reply to comment #5)
> I put in formatting: block for one or more of the screen reader vendors.
> My opinion is, keep the name "formatting". Otherwise different versions of
> Firefox will expose IA2 differently.
Practically speaking, I'm not concerned what it is called so long as it is exported on all nodes and covers display types other than "block", such as "inline", etc.

With regard to standardisation, however, I strongly believe that the "display" attribute should be implemented. It seems that the convention with IA2's object attributes is to conform to standards where possible and "display" is clearly outlined in css.

With regard to backwards compatibility, I think NVDA is the only AT using this.
However, if backwards compatibility is important for this, perhaps "display" should be implemented separately and "formatting" should be left as is.

> Even if the screen readers are just starting to update to use this, we don't
> want them to have to check for the attribute twice.
Agreed.

Comment 9

[Pete Brunet]

Can "formatting" be deprecated (noted in the docs) for 3.x and removed in 4.x?  That would give very early notice of the issue and a long time for remediation, if there is even a problem (since only NVDA seems to be using it).

Comment 10

[Aaron Leventhal]

Okay.

Comment 11

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 12

[Marco Zehe (:MarcoZ)]

Comment on attachment 327724 [details] [diff] [review]
patch

>diff --git a/accessible/tests/mochitest/test_cssattrs.html b/accessible/tests/mochitest/test_cssattrs.html
>new file mode 100644
>--- /dev/null
>+++ b/accessible/tests/mochitest/test_cssattrs.html
>@@ -0,0 +1,86 @@
>+<html>
>+<!--
>+https://bugzilla.mozilla.org/show_bug.cgi?id=439566
>+-->
>+<head>
>+  <title>nsOuterDocAccessible chrome tests</title>

nit: Change the title please. :-)

>+  <script type="application/javascript">
>+    const nsIAccessibleRetrieval = Components.interfaces.nsIAccessibleRetrieval;
>+    const nsIAccessibleRole = Components.interfaces.nsIAccessibleRole;

I believe you don't actually use the nsIRole interface, please remove the unneeded constant.

With that, r=me. Thanks!

Comment 13

[alexander :surkov (:asurkov)]

checked in with addressed comments, 97935eda2525, 2008-07-03 12:12 +0800

Comment 14

[Marco Zehe (:MarcoZ)]

Verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a1pre) Gecko/2008070603 Minefield/3.1a1pre

Comment 15

[Aaron Leventhal]

Comment on attachment 327724 [details] [diff] [review]
patch

We need this for JAWS 10 support of rich text editing.

Comment 16

[Daniel Veditz [:dveditz]]

Comment on attachment 327724 [details] [diff] [review]
patch

we need to bring in the 3.0.4 schedule, bumping to next release.

Comment 17

[Daniel Veditz [:dveditz]]

Comment on attachment 327724 [details] [diff] [review]
patch

Approved for 1.9.0.5, a=dveditz for release-drivers

Comment 18

[Marco Zehe (:MarcoZ)]

Checking in accessible/src/base/nsAccessibilityAtomList.h;
/cvsroot/mozilla/accessible/src/base/nsAccessibilityAtomList.h,v  <--  nsAccessibilityAtomList.h
new revision: 1.93; previous revision: 1.92
done
Checking in accessible/src/base/nsAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessible.cpp,v  <--  nsAccessible.cpp
new revision: 1.379; previous revision: 1.378
done
Checking in accessible/src/html/nsHyperTextAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHyperTextAccessible.cpp,v  <--  nsHyperTextAccessible.cpp
new revision: 1.118; previous revision: 1.117
done
Checking in accessible/tests/mochitest/Makefile.in;
/cvsroot/mozilla/accessible/tests/mochitest/Makefile.in,v  <--  Makefile.in
new revision: 1.23; previous revision: 1.22
done
RCS file: /cvsroot/mozilla/accessible/tests/mochitest/test_cssattrs.html,v
done
Checking in accessible/tests/mochitest/test_cssattrs.html;
/cvsroot/mozilla/accessible/tests/mochitest/test_cssattrs.html,v  <--  test_cssattrs.html
initial revision: 1.1
done

Comment 19

[Marco Zehe (:MarcoZ)]

Verified fixed on 1.9.0 branch using Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.0.5) Gecko/2008120122 Firefox/3.0.5