791916 - HTML label accessible should be created by tag name

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: Frank's testcase

It's created in nsInlineFrame (http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsInlineFrame.cpp#933) by tag name.

It prevents the following example work:
<input id="id" type="checkbox"></input>
<label for="id" style="display: block; overflow: hidden;">Label 1</label>

since the label doesn't get nsInlineFrame frame. We need to create a label accessible by tag name unconditionally.

steps to fix:
1) remove nsAccessibilityService::CreateHTMLLabelAccessible method and references to it
2) create a label accessible inside nsAccessibilityService::CreateHTMLAccessibleByMarkup

Comment 1

[saran]

hi .. I would like to help out.. Could you guide me on how to proceed ? Thanks.

Comment 2

[saran]

Based on your steps , I could do the following. Could you please tell me if I am on the right track ?Thanks.

When I did a mxr search for CreateHTMLLabelAccessible , I got 
http://mxr.mozilla.org/mozilla-central/search?string=CreateHTMLLabelAccessible&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

So according to your step 1 , should I remove the 3 occurrences ?
Also in step 2, I am not sure how to create a label accessible inside nsAccessibilityService::CreateHTMLAccessibleByMarkup.(Is it  Accessible* accessible =    new HTMLLabelAccessible(aContent, GetDocAccessible(aPresShell)); )

Comment 3

[alexander :surkov (:asurkov)]

(In reply to saran from comment #2)
> Based on your steps , I could do the following. Could you please tell me if
> I am on the right track ?Thanks.
> 
> When I did a mxr search for CreateHTMLLabelAccessible , I got 
> http://mxr.mozilla.org/mozilla-central/
> search?string=CreateHTMLLabelAccessible&find=&findi=&filter=^[^\0]*%24&hitlim
> it=&tree=mozilla-central
> 
> So according to your step 1 , should I remove the 3 occurrences ?

yes

> Also in step 2, I am not sure how to create a label accessible inside
> nsAccessibilityService::CreateHTMLAccessibleByMarkup.(Is it  Accessible*
> accessible =    new HTMLLabelAccessible(aContent,
> GetDocAccessible(aPresShell)); )

yes, you need to do that if tag name is label

Comment 4

[BDZ]

I'd like to help with this bug

Comment 5

[alexander :surkov (:asurkov)]

(In reply to BDZ from comment #4)
> I'd like to help with this bug

should I assign it to you?

Comment 6

[BDZ]

yes .Could u guide me with what I have to do?

Comment 7

[alexander :surkov (:asurkov)]

(In reply to BDZ from comment #6)
> yes .Could u guide me with what I have to do?

done. please follow comment #1 - comment#3

Comment 8

[Eitan Isaacson [:eeejay]]

(In reply to BDZ from comment #6)
> yes .Could u guide me with what I have to do?

I have a patch for this in my queue, I may re-assign this to myself. Sorry if you worked hard on a patch :(

Comment 9

[Eitan Isaacson [:eeejay]]

Attachment: Make label tag a label accessible regardless of frame type.

Comment 10

[alexander :surkov (:asurkov)]

Comment on attachment 803267 [details] [diff] [review]
Make label tag a label accessible regardless of frame type.

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

::: accessible/src/base/AccTypes.h
@@ +47,5 @@
>    /**
>     * Other accessible types.
>     */
>    eApplicationType,
> +  eHTMLLabelType,

seems like you don't need it and we can free some space for somethin else

::: accessible/src/base/nsAccessibilityService.cpp
@@ +1444,5 @@
>        new HTMLProgressMeterAccessible(aContent, document);
>      return accessible.forget();
>    }
>  
> +  if (tag == nsGkAtoms::label) {

it'd be good to move it up it a bit, at least before output and progress, it's more common than these elements

Comment 11

[alexander :surkov (:asurkov)]

(In reply to Eitan Isaacson [:eeejay] from comment #8)
> (In reply to BDZ from comment #6)
> > yes .Could u guide me with what I have to do?
> 
> I have a patch for this in my queue, I may re-assign this to myself. Sorry
> if you worked hard on a patch :(

stealing assigning status (sorry), BDZ please let us know if you;d like to work on something else.

Comment 12

[Eitan Isaacson [:eeejay]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/eaf8acdace58

Addressed issues above.

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/eaf8acdace58