The default bug view has changed. See this FAQ.

HTML label accessible should be created by tag name

RESOLVED FIXED in mozilla26

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: surkov, Assigned: eeejay)

Tracking

(Blocks: 1 bug)

unspecified
mozilla26
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
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

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

Comment 2

5 years ago
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)); )
(Reporter)

Comment 3

5 years ago
(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

4 years ago
I'd like to help with this bug
(Reporter)

Comment 5

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

should I assign it to you?

Comment 6

4 years ago
yes .Could u guide me with what I have to do?
(Reporter)

Comment 7

4 years ago
(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
(Assignee)

Comment 8

4 years ago
(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 :(
(Assignee)

Comment 9

4 years ago
Created attachment 803267 [details] [diff] [review]
Make label tag a label accessible regardless of frame type.
Attachment #803267 - Flags: review?(surkov.alexander)
(Reporter)

Comment 10

4 years ago
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+
(Reporter)

Comment 11

4 years ago
(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
(Assignee)

Comment 12

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

Addressed issues above.
https://hg.mozilla.org/mozilla-central/rev/eaf8acdace58
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.