Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XWIKI-22580: New release input fields have no text alternative #3580

Merged
merged 8 commits into from
Jan 23, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,12 @@ public void displayCustom(StringBuffer buffer, String fieldName, String prefix,
ScriptContext.ENGINE_SCOPE);
scontext.setAttribute("object", new com.xpn.xwiki.api.Object(object, context), ScriptContext.ENGINE_SCOPE);
scontext.setAttribute("type", type, ScriptContext.ENGINE_SCOPE);
/* This is a text alternative fallback to explain what the input is about.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does having an aria-label only apply to PropertyClass and StringClass only?
If we provide a default generic text alternative, it is probably better if we try to cover all cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before reaching this solution, I tried to provide a generic fix for all Property types. However, the displayEdit function is overridden for a lot of object classes. This solution only fix String properties, but a similar solution could be used to provide text alternatives for the inputs of other property types.

Since there's no straightforward way to generalize this without updating all of the classes implementations for displayEdit, I don't think I want to merge it all at once.

These changes have a wide impact, so I'd rather keep them as minimal as necessary to avoid breaking things. See for example the number of test updates needed just for what was done until now.

Moreover it doesn't seem to me like it's needed for other property types. This might already be too generalized, in practice I only need this change to the displayEdit function for

#if ($xcontext.action == "edit" || $xcontext.action == "inline")
$doc.use("ReleaseCode.ReleaseClass")
Version: $doc.display("version")
Release Managers: $doc.display("releaseManagers")
Release Date: $doc.display("releaseDate")
#else
## Note: We use the include macro below to circumvent the "nested script" execution issue...
{{include reference="ReleaseCode.ReleaseSheetHeader"/}}
{{include reference=""/}}
#end
. I could probably just have hardcoded a text alternative in this form sheet.

WDYT?

If the input has already been labelled in another way, this fallback will be ignored by Assistive Techs.
*/
scontext.setAttribute("aria-label",
localizePlainOrKey("core.model.xclass.editClassProperty.textAlternative",
this.getTranslatedPrettyName(context)), ScriptContext.ENGINE_SCOPE);

BaseProperty prop = (BaseProperty) object.safeget(fieldName);
if (prop != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ public void displayEdit(StringBuffer buffer, String name, String prefix, BaseCol
input.setID(prefix + name);
input.setSize(getSize());
input.setDisabled(isDisabled());
/* This is a text alternative fallback to explain what the input is about.
If the input has already been labelled in another way, this fallback will be ignored by Assistive Techs.
*/
input.addAttribute("aria-label", localizePlainOrKey("core.model.xclass.editClassProperty.textAlternative",
this.getTranslatedPrettyName(context)));

if (isPicker()) {
displayPickerEdit(input);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,7 @@ core.importer.saveDocumentComment=Imported from XAR
core.importer.securitySettingsChanged=Security settings have changed during the import. You will need <a href="{0}">to authenticate</a> in order to continue to administrate the wiki.
core.importer.importAsBackup=Import as backup package

core.model.xclass.editClassProperty.textAlternative=Edit the property "{0}".
core.model.xclass.deleteClassProperty.versionSummary=Removed class property "{0}"
core.model.xclass.disableClassProperty.versionSummary=Disabled class property "{0}"
core.model.xclass.enableClassProperty.versionSummary=Enabled class property "{0}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ void setDisplayMode() throws Exception
context.setDisplayMode("edit");

// We verify that the result contains a form input
assertEquals("<input size='5' id='space.page_0_prop' value='value' name='space.page_0_prop' " + "type='text'/>",
assertEquals("<input size='5' id='space.page_0_prop' aria-label='core.model.xclass.editClassProperty.textAlternative' value='value' name='space.page_0_prop' " + "type='text'/>",
document.display("prop", xcontext));

assertEquals("edit", context.getDisplayMode());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ public void display()
this.document.setSyntax(Syntax.XWIKI_2_0);

assertEquals(
"{{html clean=\"false\" wiki=\"false\"}}<input size='30' id='Space.Page_0_string' value='string' name='Space.Page_0_string' type='text'/>{{/html}}",
"{{html clean=\"false\" wiki=\"false\"}}<input size='30' id='Space.Page_0_string' aria-label='core.model.xclass.editClassProperty.textAlternative' value='string' name='Space.Page_0_string' type='text'/>{{/html}}",
this.document.display("string", "edit", this.oldcore.getXWikiContext()));

assertEquals("string", this.document.display("string", "view", this.oldcore.getXWikiContext()));
Expand Down Expand Up @@ -633,7 +633,7 @@ public void display1020()

assertEquals("string", this.document.display("string", "view", this.oldcore.getXWikiContext()));
assertEquals(
"{pre}<input size='30' id='Space.Page_0_string' value='string' name='Space.Page_0_string' type='text'/>{/pre}",
"{pre}<input size='30' id='Space.Page_0_string' aria-label='core.model.xclass.editClassProperty.textAlternative' value='string' name='Space.Page_0_string' type='text'/>{/pre}",
this.document.display("string", "edit", this.oldcore.getXWikiContext()));

assertEquals("<p>area</p>", this.document.display("area", "view", this.oldcore.getXWikiContext()));
Expand All @@ -650,7 +650,7 @@ public void displayTemplate20()

assertEquals("string", this.document.display("string", "view", this.oldcore.getXWikiContext()));
assertEquals(
"<input size='30' id='Space.Page_0_string' value='string' name='Space.Page_0_string' type='text'/>",
"<input size='30' id='Space.Page_0_string' aria-label='core.model.xclass.editClassProperty.textAlternative' value='string' name='Space.Page_0_string' type='text'/>",
this.document.display("string", "edit", this.oldcore.getXWikiContext()));

assertEquals("<p>area</p>", this.document.display("area", "view", this.oldcore.getXWikiContext()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.xwiki.localization.ContextualLocalizationManager;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.DocumentReferenceResolver;
import org.xwiki.model.reference.EntityReferenceSerializer;
Expand Down Expand Up @@ -77,6 +78,7 @@ public void before() throws Exception
this.oldCore.getMocker().registerMockComponent(EntityReferenceSerializer.TYPE_STRING, "compactwiki");
this.oldCore.getMocker().registerMockComponent(DocumentReferenceResolver.TYPE_STRING, "currentmixed");
this.oldCore.getMocker().registerMockComponent(EntityReferenceSerializer.TYPE_STRING, "local");
this.oldCore.getMocker().registerMockComponent(ContextualLocalizationManager.class);
}

/** Test the {@link PropertyClass#compareTo(PropertyClass)} method. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.xwiki.localization.ContextualLocalizationManager;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.LocalDocumentReference;

Expand Down Expand Up @@ -50,8 +51,11 @@ class StringClassTest
private XWikiURLFactory urlFactory;

@Test
void displayEdit()
void displayEdit() throws Exception
{
// Setup
this.oldCore.getMocker().registerMockComponent(ContextualLocalizationManager.class);

XWikiContext xWikiContext = this.oldCore.getXWikiContext();
xWikiContext.setURLFactory(this.urlFactory);
when(this.oldCore.getSpyXWiki()
Expand All @@ -70,14 +74,14 @@ void displayEdit()
StringBuffer stringBuffer = new StringBuffer();
stringClass.displayEdit(stringBuffer, fieldName, spaceName + "." + pageName + "_0_", baseClass,
xWikiContext);
assertEquals("<input "
assertEquals("<input aria-label='core.model.xclass.editClassProperty.textAlternative' "
+ "onfocus='new ajaxSuggest(this, &#123;script:&#34;\\/a\\/b?xpage=suggest&#38;"
+ "classname=%22%20%2B%20alert%281%29%20%2B%20%22.WebHome&#38;fieldname=test&#38;firCol=-&#38;"
+ "secCol=-&#38;&#34;, varname:&#34;input&#34;} )' "
+ "size='30' "
+ "id='&#34; + alert(1) + &#34;.WebHome_0_test' "
+ "class='suggested' "
+ "id='&#34; + alert(1) + &#34;.WebHome_0_test' "
+ "name='&#34; + alert(1) + &#34;.WebHome_0_test' "
+ "size='30' "
+ "type='text'/>", stringBuffer.toString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
'name': $id,
'value': $value,
'disabled': $disabled,
'data-format': $format
'data-format': $format,
'aria-label': $aria-label
})
#set ($picker = $field.getProperty('picker').value == 1)
#if ($picker)
Expand Down