Bug 45190 – [Gtk] A list item's number/bullet should not be a child of that list item

• Component: Accessibility
• Status: RESOLVED FIXED

[Joanmarie Diggs]

Created attachment 66529 [details]
test case

Steps to reproduce:

1. Open the attached test case.
2. Use Accerciser to examine the accessible hierarchy and text.

Expected results:
* Bullets and numbers would not be children of the list item
* Bullets and numbers would be included in the accessible text
  for the list item.

Actual results: (the inverse of the expected results)

[Mario Sanchez Prada]

I guess this changes means to change WebCore::AccessibilityRenderObject::accessibilityIsIgnored() to return *true* for those objects with roleValue() == ListMarkerRole, thus affecting all the ports.

My question then is, is this change that all the ports are willing to accept? IMHO, it doesn't make sense (neither in GTK nor in any other port) to have a a11y object just fro the  number/bullet and, in case that info was important, setting it along with the acecssible text should be enough. But I don't know what other software (like VO, for instance) expects in this case so any light on this topic would be welcome.

Joanmarie, Chris, what do you think about this?

[chris fleizach]

(In reply to comment #1)
> I guess this changes means to change WebCore::AccessibilityRenderObject::accessibilityIsIgnored() to return *true* for those objects with roleValue() == ListMarkerRole, thus affecting all the ports.
> 
> My question then is, is this change that all the ports are willing to accept? IMHO, it doesn't make sense (neither in GTK nor in any other port) to have a a11y object just fro the  number/bullet and, in case that info was important, setting it along with the acecssible text should be enough. But I don't know what other software (like VO, for instance) expects in this case so any light on this topic would be welcome.
> 
> Joanmarie, Chris, what do you think about this?

This is expected behavior on the Mac. Please do not break

[Mario Sanchez Prada]

Created attachment 66728 [details]
Patch 

(In reply to comment #2)
> [...]
> This is expected behavior on the Mac. Please do not break

Thanks for the info Chris, but at the end there's no need (fortunately) to break anything in other ports, as all the changes can be carried out for the GTK port only.

I'm now attaching a patch that would fix this stuff following Joanmarie's suggestion on IRC (thanks!). Hence, now pending for review.

[Xan Lopez]

Comment on attachment 66728 [details]
Patch 

I think this is OK. Joanie and you should work towards writing actual layout or unit tests for this stuff instead of manual testcases for each bug. Seems a waste not to do that since she's already writing one test per bug she opens most of the time.

[Mario Sanchez Prada]

Created attachment 66761 [details]
Patch + unit test

(In reply to comment #4)
> (From update of attachment 66728 [details])
> I think this is OK. Joanie and you should work towards writing actual layout or unit tests for this stuff 
> instead of manual testcases for each bug. Seems a waste not to do that since she's already writing 
> one test per bug she opens most of the time.

Understood. Now replacing the former patch with another one including the same code *plus* a new unit test to check everything is working as expected (inspired in Joanie's test case).

Btw, if you review+ this new patch don't forget to put the commit-queue+ flag as well, since I'm not a committer and I can't commit patches on my own.

Thanks for reviewing and sorry for the hassle

[Martin Robinson]

Comment on attachment 66761 [details]
Patch + unit test

View in context: https://bugs.webkit.org/attachment.cgi?id=66761&action=prettypatch

Looks sane.

[Mario Sanchez Prada]

(In reply to comment #6)
> (From update of attachment 66761 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=66761&action=prettypatch
> 
> Looks sane.

Could please someone put the commit-queue+ flag on this bug? I'm not a committer, so I couldn't get it it on my own.

Other than that, please let me know if I'm allowed to set that flag on my own. I'm not sure about that.

[Philippe Normand]

Comment on attachment 66761 [details]
Patch + unit test

Validating commit request of the r+'d patch

[Mario Sanchez Prada]

(In reply to comment #8)
> (From update of attachment 66761 [details])
> Validating commit request of the r+'d patch

Thanks!

[Eric Seidel (no email)]

Comment on attachment 66728 [details]
Patch 

Cleared Xan Lopez's review+ from obsolete attachment 66728 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[WebKit Commit Bot]

Comment on attachment 66761 [details]
Patch + unit test

Clearing flags on attachment: 66761

Committed r66964: <http://trac.webkit.org/changeset/66964>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.