Closed
Bug 762876
Opened 12 years ago
Closed 12 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•12 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•12 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•12 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•12 years ago
|
Component: Untriaged → Layout
Product: Firefox → Core
QA Contact: untriaged → layout
Updated•12 years ago
|
Component: Layout → Disability Access APIs
QA Contact: layout → accessibility-apis
Comment 3•12 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•12 years ago
|
||
Thanks, Alex, that's what I observe. Can you fix bug summary then?
Updated•12 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter | ||
Updated•12 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•12 years ago
|
||
Looks like a regression from bug 741398 but I don't know how our mochitest passes.
Depends on: 741398
Comment 6•12 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•12 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•12 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•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #642972 -
Flags: review?(dbolter)
Comment 11•12 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•12 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•12 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•12 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•12 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•12 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•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/dc3835f235cb
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Assignee | ||
Comment 18•12 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•12 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
•