Merge "[INTERNAL] ui.layout.Form: Aria role assignment"

Author: FabPete

src/sap.m/src/sap/m/TitlePropagationSupport.js

@@ -52,13 +52,15 @@ sap.ui.define(["sap/ui/core/ControlBehavior"],
 		 * @version ${version}
 		 *
 		 * @param {string} sAggregationName the name of the aggregation which should be affected
-		 * @param {object} fnGetTitleID function that would return the ID of the title
+		 * @param {function} fnGetTitleInfo function that returns the title ID as a string,
+		 *   or an object <code>{ id: string, role?: string }</code> containing the title ID and optionally the ARIA role
+		 *   of the title's container
 		 *
 		 * @private
 		 * @alias sap.m.TitlePropagationSupport
 		 * @function
 		 */
-		return function (sAggregationName, fnGetTitleID) {
+		return function (sAggregationName, fnGetTitleInfo) {
 			// "this" is the prototype now when called with call()
 
 			// Ensure only Elements are enhanced
@@ -76,22 +78,32 @@ sap.ui.define(["sap/ui/core/ControlBehavior"],
 
 				var oAggregation = this.getMetadata().getAggregation(sAggregationName),
 					aContent = oAggregation && oAggregation.get(this),
-					sTitleID = fnGetTitleID && fnGetTitleID.call(this),
+					vTitleInfo = fnGetTitleInfo && fnGetTitleInfo.call(this),
+					oTitleData,
 					oItem;
 
 				// Note: in case accessibility mode is off we don't need the propagation
-				if (!ControlBehavior.isAccessibilityEnabled() || !sTitleID || !aContent
+				if (!ControlBehavior.isAccessibilityEnabled() || !vTitleInfo || !aContent
 					|| aContent.length === 0) {
 						return false;
 				}
 
+				// Normalize: string -> { id: string }, object stays as-is
+				if (typeof vTitleInfo === "string") {
+					oTitleData = { id: vTitleInfo };
+				} else if (vTitleInfo.id) {
+					oTitleData = vTitleInfo;
+				} else {
+					return false;
+				}
+
 				// Propagate title ID only to first control in the content
 				oItem = aContent[0];
 				if (oItem && oItem._suggestTitleId && oItem.isA([
 					"sap.ui.layout.form.SimpleForm",
 					"sap.ui.layout.form.Form",
 					"sap.ui.comp.smartform.SmartForm"])) {
-						oItem._suggestTitleId(sTitleID);
+						oItem._suggestTitleId(oTitleData);
 						return true;
 				}
 				return false;

src/sap.m/test/sap/m/qunit/TitlePropagationSupport.qunit.js

@@ -113,8 +113,8 @@ sap.ui.define([
 			if (!bNegative) {
 				assert.strictEqual(oSuggestSpy.callCount, 1,
 						"Suggest method is called once for control of type " + sType);
-				assert.ok(oSuggestSpy.calledWith(sTitleID),
-						"Suggest method on " + sType + " is called with the expected title ID: " +
+				assert.ok(oSuggestSpy.calledWithMatch({id: sTitleID}),
+						"Suggest method on " + sType + " is called with object containing the expected title ID: " +
 						sTitleID);
 			} else {
 				assert.strictEqual(oSuggestSpy.callCount, 0,

src/sap.ui.layout/src/sap/ui/layout/form/Form.js

@@ -356,13 +356,20 @@ sap.ui.define([
 	 * Method used to propagate the <code>Title</code> control ID of a container control
 	 * (like a <code>Dialog</code> control) to use it as aria-label in the <code>Form</code>.
 	 * So the <code>Form</code> must not have an own title.
-	 * @param {string} sTitleID <code>Title</code> control ID
+	 * @param {string | object} sTitleID <code>Title</code> control ID or object <code>{ id: string, role?: string }</code> containing title ID and aria role
 	 * @private
 	 * @return {this} Reference to <code>this</code> to allow method chaining
 	 */
 	Form.prototype._suggestTitleId = function (sTitleID) {
+		if (typeof sTitleID === "string") {
+			this._sSuggestedTitleId = sTitleID;
+		} else if (sTitleID && sTitleID.id) {
+			this._sSuggestedTitleId = sTitleID.id;
+			if (sTitleID.role) {
+				this._sSuggestedTitleAriaRole = sTitleID.role;
+			}
+		}
 
-		this._sSuggestedTitleId = sTitleID;
 		if (this.getDomRef()) {
 			this.invalidate();
 		}

src/sap.ui.layout/src/sap/ui/layout/form/FormRenderer.js

@@ -38,6 +38,14 @@ sap.ui.define([
 		const oLayout = oForm.getLayout();
 		const mAriaProps = {role: oLayout && oLayout.hasLabelledContainers(oForm) ? "region" : "form"};
 
+		const sTitleID = oLayout?.getRenderer().getTitleId(oForm) || oForm._sSuggestedTitleId;
+		const bUsingSuggestedTitle = !oLayout?.getRenderer().getTitleId(oForm) && oForm._sSuggestedTitleId;
+
+		if (bUsingSuggestedTitle && oForm._sSuggestedTitleAriaRole === "region" && mAriaProps.role === "region") {
+			// Do not render a role, if the title already defines a region and the form would do the same
+			delete mAriaProps.role;
+		}
+
 		// write only a DIV for the form and let the layout render the rest
 		rm.openStart("div", oForm)
 			.class("sapUiForm")
@@ -63,7 +71,6 @@ sap.ui.define([
 			rm.attr('title', oForm.getTooltip_AsString());
 		}
 
-		const sTitleID = oLayout?.getRenderer().getTitleId(oForm) || oForm._sSuggestedTitleId;
 		if (sTitleID) {
 			mAriaProps["labelledby"] = {value: sTitleID, append: true};
 		}

src/sap.ui.layout/test/sap/ui/layout/qunit/form/Form.qunit.js

@@ -21,7 +21,10 @@ sap.ui.define([
 	"sap/m/Title",
 	"sap/m/Label",
 	"sap/m/Input",
-	"sap/ui/qunit/utils/nextUIUpdate"
+	"sap/ui/qunit/utils/nextUIUpdate",
+	"sap/uxap/ObjectPageLayout",
+	"sap/uxap/ObjectPageSection",
+	"sap/uxap/ObjectPageSubSection"
 ],
 	function(
 		Element,
@@ -44,7 +47,10 @@ sap.ui.define([
 		mTitle,
 		Label,
 		Input,
-		nextUIUpdate
+		nextUIUpdate,
+		ObjectPageLayout,
+		ObjectPageSection,
+		ObjectPageSubSection
 	) {
 	"use strict";
 
@@ -243,6 +249,16 @@ sap.ui.define([
 		assert.equal(jQuery("#F1").attr("aria-labelledby"), "X T1", "aria-labelledby points to AriaLabel and Title");
 	});
 
+	QUnit.test("_suggestTitleId with role: region", async function(assert) {
+		// Setup: Form without title, receives suggested title with role "region" from parent
+		oForm._suggestTitleId({ id: "parentTitle", role: "region" });
+		await nextUIUpdate();
+
+		assert.equal(jQuery("#F1").attr("aria-labelledby"), "parentTitle", "aria-labelledby points to parent title");
+
+		assert.notEqual(jQuery("#F1").attr("role"), "region", "Form does not render role='region' when parent has region role");
+	});
+
 	QUnit.module("FormContainer", {
 		beforeEach: initTest,
 		afterEach: afterTest
@@ -803,4 +819,51 @@ sap.ui.define([
 		assert.notOk(oFormLayout.renderControlsForSemanticElement(), "no control rendering supported");
 	});
 
+	QUnit.module("Form in ObjectPageSubSection", {
+		beforeEach: async function() {
+			oFormLayout = new FormLayout("FL1");
+			// Create a FormContainer with a title so Form would render role="region"
+			const oFormContainer = new FormContainer("FC1", {
+				title: new Title("FCT1", { text: "Container Title" })
+			});
+			oForm = new Form("F1", {
+				layout: oFormLayout,
+				formContainers: [oFormContainer]
+			});
+			this.oObjectPage = new ObjectPageLayout("OP1", {
+				sections: [new ObjectPageSection("OPS1", {
+					title: "Section Title",
+					subSections: [new ObjectPageSubSection("OPSS1", {
+						title: "SubSection Title",
+						blocks: [oForm]
+					})]
+				})]
+			}).placeAt("qunit-fixture");
+			await nextUIUpdate();
+		},
+		afterEach: function() {
+			this.oObjectPage.destroy();
+			oForm.destroy();
+			oFormLayout.destroy();
+			oForm = undefined;
+			oFormLayout = undefined;
+		}
+	});
+
+	QUnit.test("Form with labelled container does not render duplicate region role", function(assert) {
+		assert.ok(oFormLayout.hasLabelledContainers(oForm), "Form has labelled containers");
+
+		const sFormRole = jQuery("#F1").attr("role");
+		assert.notEqual(sFormRole, "region", "Form does not have role='region' when inside ObjectPageSubSection");
+	});
+
+	QUnit.test("Form with own title still renders region role", async function(assert) {
+		const oTitle = new Title("FT1", { text: "Form Title" });
+		oForm.setTitle(oTitle);
+		await nextUIUpdate();
+
+		const sFormRole = jQuery("#F1").attr("role");
+		assert.equal(sFormRole, "region", "Form has role='region' when it has its own title");
+	});
+
 });

src/sap.uxap/src/sap/uxap/ObjectPageSubSection.js

@@ -196,7 +196,11 @@ sap.ui.define([
 
 	// Add Title Propagation Support
 	TitlePropagationSupport.call(ObjectPageSubSection.prototype, "blocks", function () {
-		return this._getTitleDomId();
+		var sTitleDomId = this._getTitleDomId();
+		if (!sTitleDomId) {
+			return false;
+		}
+		return { id: sTitleDomId, role: "region" };
 	});