Closed Bug 791916 Opened 10 years ago Closed 9 years ago

HTML label accessible should be created by tag name

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: surkov, Assigned: eeejay)

References

(Blocks 1 open bug)

Details

(Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=c++])

Attachments

(2 files)

Attached file 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
hi .. I would like to help out.. Could you guide me on how to proceed ? Thanks.
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)); )
(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
I'd like to help with this bug
(In reply to BDZ from comment #4)
> I'd like to help with this bug

should I assign it to you?
yes .Could u guide me with what I have to do?
(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
Assignee: nobody → brandondsz26
(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 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
Attachment #803267 - Flags: review?(surkov.alexander) → review+
(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.
Assignee: brandondsz26 → eitan
https://hg.mozilla.org/mozilla-central/rev/eaf8acdace58
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.