Last Comment Bug 791916 - HTML label accessible should be created by tag name
: HTML label accessible should be created by tag name
Status: RESOLVED FIXED
[good first bug][mentor=surkov.alexan...
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla26
Assigned To: Eitan Isaacson [:eeejay]
:
: alexander :surkov
Mentors:
Depends on:
Blocks: 513662
  Show dependency treegraph
 
Reported: 2012-09-17 20:47 PDT by alexander :surkov
Modified: 2013-09-12 04:12 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Frank's testcase (12.44 KB, text/html)
2012-09-17 20:47 PDT, alexander :surkov
no flags Details
Make label tag a label accessible regardless of frame type. (3.57 KB, patch)
2013-09-11 13:31 PDT, Eitan Isaacson [:eeejay]
surkov.alexander: review+
Details | Diff | Splinter Review

Description User image alexander :surkov 2012-09-17 20:47:54 PDT
Created attachment 662014 [details]
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 User image saran 2012-10-01 23:20:15 PDT
hi .. I would like to help out.. Could you guide me on how to proceed ? Thanks.
Comment 2 User image saran 2012-10-01 23:42:31 PDT
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 User image alexander :surkov 2012-10-04 03:11:50 PDT
(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 User image BDZ 2013-08-21 05:02:57 PDT
I'd like to help with this bug
Comment 5 User image alexander :surkov 2013-08-21 05:52:54 PDT
(In reply to BDZ from comment #4)
> I'd like to help with this bug

should I assign it to you?
Comment 6 User image BDZ 2013-08-21 07:51:18 PDT
yes .Could u guide me with what I have to do?
Comment 7 User image alexander :surkov 2013-08-21 09:43:57 PDT
(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 User image Eitan Isaacson [:eeejay] 2013-09-11 13:23:25 PDT
(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 User image Eitan Isaacson [:eeejay] 2013-09-11 13:31:48 PDT
Created attachment 803267 [details] [diff] [review]
Make label tag a label accessible regardless of frame type.
Comment 10 User image alexander :surkov 2013-09-11 16:03:01 PDT
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 User image alexander :surkov 2013-09-11 16:04:26 PDT
(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 User image Eitan Isaacson [:eeejay] 2013-09-11 17:07:26 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/eaf8acdace58

Addressed issues above.
Comment 13 User image Ed Morley [:emorley] 2013-09-12 04:12:31 PDT
https://hg.mozilla.org/mozilla-central/rev/eaf8acdace58

Note You need to log in before you can comment on or make changes to this bug.