Closed
Bug 762876
Opened 13 years ago
Closed 13 years ago
IA2_STATE_HORIZONTAL state is set for “aria-orientation=vertical”
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: jeckhardt, Assigned: tbsaunde)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
7.35 KB,
patch
|
davidb
:
review+
|
Details | Diff | Splinter Review |
10.32 KB,
patch
|
davidb
:
review+
|
Details | Diff | Splinter Review |
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•13 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•13 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•13 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•13 years ago
|
Component: Untriaged → Layout
Product: Firefox → Core
QA Contact: untriaged → layout
Updated•13 years ago
|
Component: Layout → Disability Access APIs
QA Contact: layout → accessibility-apis
Comment 3•13 years ago
|
||
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•13 years ago
|
||
Thanks, Alex, that's what I observe. Can you fix bug summary then?
Updated•13 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter | ||
Updated•13 years ago
|
Summary: “aria-orientation” state is not mapped to the IA2_STATE_HORIZONTAL → IA2_STATE_HORIZONTAL state is set for “aria-orientation=vertical”
Comment 5•13 years ago
|
||
Looks like a regression from bug 741398 but I don't know how our mochitest passes.
Depends on: 741398
Comment 6•13 years ago
|
||
(In reply to alexander :surkov from comment #2)
> I see that aria-orientation="vertical" are mapped to vertical and horizontal
> states
Aha.
Comment 7•13 years ago
|
||
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•13 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.
Assignee | ||
Comment 10•13 years ago
|
||
Assignee | ||
Updated•13 years ago
|
Attachment #642972 -
Flags: review?(dbolter)
Comment 11•13 years ago
|
||
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•13 years ago
|
||
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 13•13 years ago
|
||
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•13 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•13 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•13 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).
Comment 17•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Assignee | ||
Comment 18•13 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
Comment 19•13 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.
Description
•