Improving accessability - added aria presentation role for layout tables

[Lonzak]

According to accessibility rules layout tables exist merely to position content visually and should not be used in HTML5.
Screen readers may interpret them as data tables (i.e., announcing column and row numbers) etc). To change that in GWT would be a bigger task. So as a minor improvement those tables should be marked as layout tables. This can be done by assigning the role="presentation" to ensure it is not identified as a table to screen reader users.

[niloc132]

Thanks for the patch, a few thoughts:

• consider spelling out UA? In the context of browsers and JS, it usually means "user agent", and I think you probably mean something else like usability/accessibility? Substituting "user agent"(e.g. "the browser") doesn't make sense in your title or description, and I don't see a obvious definition searching for the term in the context of dom and aria.

[Lonzak]

UA stands for Universal Accessibility but as always abbreviations mean something different in different contexts. Just accessibility is fine.
Strange with the blank after comment // I did run 'ant compile.tests checkstyle' as it is described in the readme.md and everything was fine. But no problem - I fixed it.

I also changed the code to use 'Roles' class which is much better. Thanks for the pointer.

[niloc132]

UA stands for Universal Accessibility but as always abbreviations mean something different in different contexts. Just accessibility is fine. Strange with the blank after comment // I did run 'ant compile.tests checkstyle' as it is described in the readme.md and everything was fine. But no problem - I fixed it.

I also changed the code to use 'Roles' class which is much better. Thanks for the pointer.

I think the "space after //" shows up as a warning, not an error, so it won't kill your build, but it will get logged if you go hunting for it. The CI run will log it at least (annotations are currently broken..), but still need a human to point it out.

Thanks!

[jnehlmeier]

Just two small things, otherwise it looks good.

[niloc132]

UA stands for Universal Accessibility but as always abbreviations mean something different in different contexts. Just accessibility is fine. Strange with the blank after comment // I did run 'ant compile.tests checkstyle' as it is described in the readme.md and everything was fine. But no problem - I fixed it.

I also changed the code to use 'Roles' class which is much better. Thanks for the pointer.

I think the "space after //" shows up as a warning, not an error, so it won't kill your build, but it will get logged if you go hunting for it. The CI run will log it at least (annotations are currently broken..), but still need a human to point it out.

Thanks!

[niloc132]

Looks like this is failing tests (verified in github actions by pushing to aria-test on my fork):

    [junit] Running com.google.gwt.user.UiSuite
    [junit] Tests run: 1008, Failures: 1, Errors: 38, Skipped: 0, Time elapsed: 143.649 sec
    [junit] Test com.google.gwt.user.UiSuite FAILED

https://github.com/niloc132/gwt/actions/runs/14194633693
One of the stack traces:

java.lang.AssertionError: Element cannot be null.
	at java.lang.Throwable.Throwable(Throwable.java:73)
	at java.lang.Error.Error(Error.java:30)
	at java.lang.AssertionError.AssertionError(AssertionError.java:51)
	at com.google.gwt.aria.client.RoleImpl.set(RoleImpl.java:219)
	at com.google.gwt.user.client.ui.TreeItem$TreeItemImpl.initializeClonableElements(TreeItem.java:92)
	at com.google.gwt.user.client.ui.TreeItem$TreeItemImpl.TreeItem$TreeItemImpl(TreeItem.java:65)
	at com.google.gwt.user.client.ui.TreeItem.$clinit(TreeItem.java:255)
	at com.google.gwt.user.client.ui.TreeItem.TreeItem(TreeItem.java:307)
	at com.google.gwt.user.client.ui.Tree.init(Tree.java:1088)
	at com.google.gwt.user.client.ui.Tree.Tree(Tree.java:204)
	at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl$Widgets.build_myTree(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:2162)
	at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl$Widgets.get_myTree(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:2158)
	at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl$Widgets.build_f_HTMLPanel1(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:1350)
	at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl$Widgets.get_f_HTMLPanel1(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:1209)
	at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl$Widgets.build_root(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:1078)
	at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl$Widgets.get_root(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:1070)
	at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.createAndBindUi(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:185)
	at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.createAndBindUi(UiBinderJsInteropTest_MyWidgetBasedUi_BinderImpl.java:182)
	at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest$MyWidgetBasedUi.init(UiBinderJsInteropTest.java:40)
	at com.google.gwt.uibinder.test.client.WidgetBasedUi.WidgetBasedUi(WidgetBasedUi.java:277)
	at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest$MyWidgetBasedUi.UiBinderJsInteropTest$MyWidgetBasedUi(UiBinderJsInteropTest.java:30)
	at com.google.gwt.uibinder.test.client.UiBinderJsInteropTest.testJsElementTypes(UiBinderJsInteropTest.java:45)

From looking at TreeItem.java, you are calling Roles.getPresentationRole().set(BASE_BARE_ELEM) before BASE_BARE_ELEM was first assigned in initializeClonableElements().

I did not check other stack traces.

[niloc132]

Tests still seem to be failing
https://github.com/niloc132/gwt/actions/runs/14759920702

It appears that the tests aren't testing what they should be - for example, menubar.init() is creating a <table> and appending to the outer <div> which already (correctly?) has the menubar role applied? Instead, you want to be finding the table tag inside the div, and testing that.

I'd suggest running the tests locally, or turn on actions on your fork so that you get these results yourself (since you're pushing to main).

The quickest test command locally will be to cd user and run ant test.draft.htmlunit '-Dgwt.junit.testcase.includes=**/UiSuite.class', to just run the suite that handles all of the tests you've changed.

[zbynek]

@Lonzak please merge https://github.com/Lonzak/gwt/pull/1 to fix the tests

[Lonzak]

sorry for the delay ...working on the junit part (running the test takes 4-6hours) so had to post pone this

[Lonzak]

Ok so everything should be ok now...?

[niloc132]

Approved, once we get nightlies back on track let's land this.

Passing nightly build: https://github.com/niloc132/gwt/actions/runs/16323934154