The default bug view has changed. See this FAQ.

IA2_STATE_HORIZONTAL state is set for “aria-orientation=vertical”

RESOLVED FIXED in mozilla17

Status

()

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

People

(Reporter: Jost Eckhardt, Ai Squared, Assigned: tbsaunde)

Tracking

(Blocks: 1 bug)

13 Branch
mozilla17
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20100101 Firefox/13.0
Build ID: 20120531155942

Steps to reproduce:

Run a test case with AccProbe

http://www.aisquared.com/testdocs/Aria/AriaInvalid.html 


Actual results:

aria-invalid is not exposed as text attribute


Expected results:

aria-invalid should be exposed as text attribute
(Reporter)

Updated

5 years ago
Summary: “aria-invalid” state is mapped to the IA2_STATE_INVALID_ENTRY, but “aria-invalid” value is not exposed as a text attribute → “aria-orientation” state is not mapped to the IA2_STATE_HORIZONTAL
(Reporter)

Comment 1

5 years ago
Sorry, I got it mixed up. The aria-invalid is an existing issue: 445510

this is about aria-orientation

aria-orientation should be mapped to IA2_STATE_HORIZONTAL

The test case is: http://www.aisquared.com/testdocs/Aria/AriaOrientation.html

Comment 2

5 years ago
(In reply to Jost Eckhardt, Ai Squared from comment #1)

> aria-orientation should be mapped to IA2_STATE_HORIZONTAL
> 
> The test case is: http://www.aisquared.com/testdocs/Aria/AriaOrientation.html

I see that aria-orientation="vertical" are mapped to vertical and horizontal states which is a bug. Bug as far as I can see by accprobe aria-orientation="horizontal" is mapped to horizontal state. Can you give me exact steps to reproduce please?

Updated

5 years ago
Component: Untriaged → Layout
Product: Firefox → Core
QA Contact: untriaged → layout

Updated

5 years ago
Component: Layout → Disability Access APIs
QA Contact: layout → accessibility-apis

Updated

5 years ago
Blocks: 343213
Accordingly to the spec(http://www.w3.org/TR/wai-aria-implementation/#mapping_state-property_table) user agents must set IA2_STATE_HORIZONTAL attribute if aria-orientation="horizontal" and clear it if aria-orientation="vertical" (aria-orientation="vertical" is not mapped to the IA2_STATE_VERTICAL). 
Steps to repro:
a)Test case #1 (http://www.aisquared.com/testdocs/Aria/AriaOrientation.html) presents three items that have vertical orientation (aria-orientation="vertical"), but AccProbe reports the following states: [vertical, ...., horizontal];
b)Test case #3  (http://www.aisquared.com/testdocs/Aria/AriaOrientation.html) presents an ARIA scrollbar that have no aria-orientation attribute defined. AccProbe reports that the orientation of the item is set to horizontal (IA2_STATE_HORIZONTAL is set), but, accordingly to the spec., aria-orientation="vertical" is a default orientation for an aria scroll bar.

Comment 4

5 years ago
Thanks, Alex, that's what I observe. Can you fix bug summary then?

Updated

5 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Updated

5 years ago
Summary: “aria-orientation” state is not mapped to the IA2_STATE_HORIZONTAL → IA2_STATE_HORIZONTAL state is set for “aria-orientation=vertical”
Looks like a regression from bug 741398 but I don't know how our mochitest passes.
Depends on: 741398
(In reply to alexander :surkov from comment #2)
> I see that aria-orientation="vertical" are mapped to vertical and horizontal
> states

Aha.
I'm a little nervous that we rely on memory alignment for the mValues (when calling FindAttrValueIn in our state map code).
(Assignee)

Comment 8

5 years ago
(In reply to David Bolter [:davidb] from comment #7)
> I'm a little nervous that we rely on memory alignment for the mValues (when
> calling FindAttrValueIn in our state map code).

why? I'm pretty sure the spec says that's legit.
Trevor can you take this one?
Assignee: nobody → trev.saunders
(Assignee)

Comment 10

5 years ago
Created attachment 642972 [details] [diff] [review]
bug 762876 - patch
(Assignee)

Updated

5 years ago
Attachment #642972 - Flags: review?(dbolter)
Comment on attachment 642972 [details] [diff] [review]
bug 762876 - patch

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

looks right.
Attachment #642972 - Flags: review?(dbolter) → review+
(Assignee)

Comment 12

5 years ago
Created attachment 644487 [details] [diff] [review]
fix some tests

these tests were cheked in doing this with no reason given.  It doesn't seem terribly useful to say links are "horizontal", so I'm proposing we just change them.
Attachment #644487 - Flags: review?(dbolter)
Comment on attachment 644487 [details] [diff] [review]
fix some tests

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

If removing this state from links is wrong, I don't want to be right. r=me.
Attachment #644487 - Flags: review?(dbolter) → review+

Comment 14

5 years ago
Comment on attachment 642972 [details] [diff] [review]
bug 762876 - patch

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

::: accessible/src/base/ARIAStateMap.cpp
@@ +235,5 @@
> +      } else {
> +        NS_ASSERTION(!(*aState & (states::HORIZONTAL | states::VERTICAL)),
> +                     "orientation state on role with default aria-orientation!");
> +        *aState |= GetRoleMap(aElement)->Is(nsGkAtoms::scrollbar) ?
> +          states::VERTICAL : states::HORIZONTAL;

it should be part of map

::: accessible/src/generic/Accessible.cpp
@@ +678,4 @@
>      if (mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::popup))
>        state |= states::HASPOPUP;
>  
> +    const nsStyleXUL *xulStyle = frame->GetStyleXUL();

type* name

@@ +678,5 @@
>      if (mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::popup))
>        state |= states::HASPOPUP;
>  
> +    const nsStyleXUL *xulStyle = frame->GetStyleXUL();
> +    if (xulStyle && frame->IsBoxFrame()) {

please an example of XUL element which is not box frame
(Assignee)

Comment 15

5 years ago
(In reply to alexander :surkov from comment #14)
> Comment on attachment 642972 [details] [diff] [review]
> bug 762876 - patch
> 
> Review of attachment 642972 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/base/ARIAStateMap.cpp
> @@ +235,5 @@
> > +      } else {
> > +        NS_ASSERTION(!(*aState & (states::HORIZONTAL | states::VERTICAL)),
> > +                     "orientation state on role with default aria-orientation!");
> > +        *aState |= GetRoleMap(aElement)->Is(nsGkAtoms::scrollbar) ?
> > +          states::VERTICAL : states::HORIZONTAL;
> 
> it should be part of map

I'm not sure what you mean all this function has access to is the eStateRule, perhaps we should pase more, but currently we don't

> ::: accessible/src/generic/Accessible.cpp
> @@ +678,4 @@
> >      if (mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::popup))
> >        state |= states::HASPOPUP;
> >  
> > +    const nsStyleXUL *xulStyle = frame->GetStyleXUL();
> 
> type* name

arg, I hate that I don't notice those when I just copy stuff around.
 
> @@ +678,5 @@
> >      if (mContent->HasAttr(kNameSpaceID_None, nsGkAtoms::popup))
> >        state |= states::HASPOPUP;
> >  
> > +    const nsStyleXUL *xulStyle = frame->GetStyleXUL();
> > +    if (xulStyle && frame->IsBoxFrame()) {
> 
> please an example of XUL element which is not box frame

no idea, I think it was because bz said just IsXUL() was wrong and suggested that.

Comment 16

5 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #15)

> > it should be part of map
> 
> I'm not sure what you mean all this function has access to is the
> eStateRule, perhaps we should pase more, but currently we don't

each entry has an option of default state, see http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsARIAMap.cpp#413 

> > type* name
> 
> arg, I hate that I don't notice those when I just copy stuff around.

if you do follow up then please fix it as well.

> > please an example of XUL element which is not box frame
> 
> no idea, I think it was because bz said just IsXUL() was wrong and suggested
> that.

in either way you put it under IsXUL() what could make bz comment irrelevant (in case if IsBoxFrame() returns true for some HTML).
https://hg.mozilla.org/mozilla-central/rev/dc3835f235cb
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
(Assignee)

Comment 18

5 years ago
(In reply to alexander :surkov from comment #16)
> (In reply to Trevor Saunders (:tbsaunde) from comment #15)
> 
> > > it should be part of map
> > 
> > I'm not sure what you mean all this function has access to is the
> > eStateRule, perhaps we should pase more, but currently we don't
> 
> each entry has an option of default state, see
> http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsARIAMap.
> cpp#413

true, but that doesn't quiet work because at some point you need to clear the default state if the non default state is set explicitly.

> > > please an example of XUL element which is not box frame

nsXULLabelFrame?

> > no idea, I think it was because bz said just IsXUL() was wrong and suggested
> > that.
> 
> in either way you put it under IsXUL() what could make bz comment irrelevant
> (in case if IsBoxFrame() returns true for some HTML).

I'm not sure

Updated

5 years ago
Depends on: 776472

Comment 19

5 years ago
(In reply to Trevor Saunders (:tbsaunde) from comment #18)
> (In reply to alexander :surkov from comment #16)
> > (In reply to Trevor Saunders (:tbsaunde) from comment #15)
> > 
> > > > it should be part of map
> > > 
> > > I'm not sure what you mean all this function has access to is the
> > > eStateRule, perhaps we should pase more, but currently we don't
> > 
> > each entry has an option of default state, see
> > http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsARIAMap.
> > cpp#413
> 
> true, but that doesn't quiet work because at some point you need to clear
> the default state if the non default state is set explicitly.

I meant, stuffs like GetRoleMap(aElement)->Is(nsGkAtoms::scrollbar) should be replaced by default state.
You need to log in before you can comment on or make changes to this bug.