Closed Bug 1031184 Opened 7 years ago Closed 5 years ago

Introduce a NativeDescription method to more easily customize accessible descriptions for various classes

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: MarcoZ, Assigned: tephra, Mentored)

References

Details

(Whiteboard: [good first bug])

Attachments

(1 file)

Spun off bug 670083 and my initial patches there. The idea is to pull class or element specific implementations for the accessible description out into methods similar to what was done for NativeName methods.
Look in the current patch for bug 670083 how the implementations for xul:description is moved out of Accessible::Description and a new method is created.
Mentor: marco.zehe
Whiteboard: [good first bug]
I would like to take this one.
OK, go right ahead! :)
Assignee: nobody → eric
Status: NEW → ASSIGNED
Eric, are you still planning to work on this bug at all? There hasn't been any activity since you decided to pick it up.
Flags: needinfo?(eric)
(In reply to Marco Zehe (:MarcoZ) from comment #4)
> Eric, are you still planning to work on this bug at all? There hasn't been
> any activity since you decided to pick it up.

Hello Marco!

So sorry for not updating, I went away from contributing for a while due to life but just got back into it.
I'll love to take a swing at it this weekend if that is ok with you?

Otherwise you are free to assign it to anyone else.

Again sorry for not updating...
Flags: needinfo?(eric)
Hi Eric, if you like, why not? We need to move this forward now due to changed circumstances, so if you're willing to help with this, it's definitely appreciated!

Look forward to a patch, then! Let me know if you need any additional information.
(In reply to Marco Zehe (:MarcoZ) from comment #6)
> Hi Eric, if you like, why not? We need to move this forward now due to
> changed circumstances, so if you're willing to help with this, it's
> definitely appreciated!
> 
> Look forward to a patch, then! Let me know if you need any additional
> information.

Great :) 

I've looked over the code and patch and not sure if I understand entirely what need to be done.
Is it a matter of taking the patch in bug 670083 and doing what was done there for xul:description or is it to do that + the other class/element specific descriptions?
Flags: needinfo?(mzehe)
(In reply to eric from comment #7)
> Is it a matter of taking the patch in bug 670083 and doing what was done
> there for xul:description or is it to do that + the other class/element
> specific descriptions?
This bug is about the part of the patch that deals with xul:description. In part 2, you can turn other Description() methods into NativeDescription without changing any of the logic.

Once all that is in place, this bug here will be called fixed. Follow-up bugs deal with specific changes, like the one in bug 670083, and others that might be found when we check the current description calculation against the specification/algorithm that was being agreed on at the W3C..
Flags: needinfo?(mzehe)
Attached patch bug1031184.patchSplinter Review
First shot at a patch.
Attachment #8755232 - Flags: review?(mzehe)
Comment on attachment 8755232 [details] [diff] [review]
bug1031184.patch

r=me.
Attachment #8755232 - Flags: review?(mzehe) → review+
Thanks Marco!

Could you push to try since I don't have try access?
Flags: needinfo?(mzehe)
(In reply to eric from comment #11)
> Could you push to try since I don't have try access?
Done (see above). For future bugs, you may want to switch to using MozReview for review requests and try pushes. It can even auto-land for you after you got reviews, and a try run was successful. More info: http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html

Once the try run is successful for this bug, I will land it for you, with the commit message adjusted to show me as the reviewer.
Flags: needinfo?(mzehe)
(In reply to Marco Zehe (:MarcoZ) from comment #13)
> (In reply to eric from comment #11)
> > Could you push to try since I don't have try access?
> Done (see above). For future bugs, you may want to switch to using MozReview
> for review requests and try pushes. It can even auto-land for you after you
> got reviews, and a try run was successful. More info:
> http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html
> 
> Once the try run is successful for this bug, I will land it for you, with
> the commit message adjusted to show me as the reviewer.

Will do! (Had somehow gotten the impression that you needed try access for MozReview).

Thanks for the help Marco!
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe1a7608cfd77794732ce6d1f9a2ee7cc33e213f
Bug 1031184 - Introduce a NativeDescription method to more easily customize accessible descriptions for various classes, r=MarcoZ
(In reply to eric from comment #14)
> Will do! (Had somehow gotten the impression that you needed try access for
> MozReview).
Sorry, my bad, yes, you do need level 1 commit access to push to review board. Sorry, my bad! It might be worth getting that if you plan to contribute more in the future. Makes things easier for sure.
(In reply to Marco Zehe (:MarcoZ) from comment #16)
> (In reply to eric from comment #14)
> > Will do! (Had somehow gotten the impression that you needed try access for
> > MozReview).
> Sorry, my bad, yes, you do need level 1 commit access to push to review
> board. Sorry, my bad! It might be worth getting that if you plan to
> contribute more in the future. Makes things easier for sure.

No worries :)
Not really sure how to get try access, but I'm certainly planning on contributing as much as I can :)
(In reply to eric from comment #17)
> Not really sure how to get try access, but I'm certainly planning on
> contributing as much as I can :)
https://www.mozilla.org/en-US/about/governance/policies/commit/ For a start, level 1 should suffice and is easiest to get.
https://hg.mozilla.org/mozilla-central/rev/fe1a7608cfd7
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.