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
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
(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?
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.
Thanks, Alex, that's what I observe. Can you fix bug summary then?
Looks like a regression from bug 741398 but I don't know how our mochitest passes.
(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).
(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?
Created attachment 642972 [details] [diff] [review] bug 762876 - patch
Comment on attachment 642972 [details] [diff] [review] bug 762876 - patch Review of attachment 642972 [details] [diff] [review]: ----------------------------------------------------------------- looks right.
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.
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.
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
(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.
(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
(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
(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.
or
