Fix Form#onFormSubmitted not considering if submitter is disabled or invisible

wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/FormSubmitTest.java

@@ -16,21 +16,25 @@
  */
 package org.apache.wicket.markup.html.form;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.fail;
 
 import org.apache.wicket.Component;
+import org.apache.wicket.WicketRuntimeException;
 import org.apache.wicket.core.request.handler.ListenerInvocationNotAllowedException;
 import org.apache.wicket.markup.html.form.NestedFormsPage.NestableForm;
 import org.apache.wicket.util.tester.FormTester;
 import org.apache.wicket.util.tester.WicketTestCase;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 
 /**
  * Please see <a href="http://cwiki.apache.org/WICKET/nested-forms.html">"Nested Forms"</a> for more
  * details on nested forms
- * 
+ *
  * @author Gerolf Seitz
  */
 class FormSubmitTest extends WicketTestCase
@@ -46,7 +50,7 @@ class FormSubmitTest extends WicketTestCase
 	@BeforeEach
 	void before()
 	{
-		tester.startPage(new NestedFormsPage());
+		tester.startPage(new NestedFormsPage(true, true));
 		page = (NestedFormsPage)tester.getLastRenderedPage();
 		outerForm = (NestableForm)page.get("outerForm");
 		middleForm = (NestableForm)page.get("outerForm:middleForm");
@@ -273,13 +277,104 @@ void errorOnInnerFormDisabledMiddleFormSubmitOuterForm()
 		assertOnErrorCalled(false, false, false);
 	}
 
+	@Nested
+	class WhenSubmitterInvisibleFormSubmitTest
+	{
+
+		@BeforeEach
+		void before()
+		{
+			tester.startPage(new NestedFormsPage(false, true));
+		}
+
+		@Test
+		void doNotSubmitInvisibleButtons_middleForm()
+		{
+			assertEnabledState(true, true, true);
+
+			FormTester formTester = tester.newFormTester("outerForm:middleForm");
+			var message = assertThrows(WicketRuntimeException.class,
+				() -> formTester.submit("submit")).getMessage();
+			assertThat(message).contains("is not visible");
+		}
+
+		@Test
+		void doNotSubmitInvisibleButtons_outerForm()
+		{
+			assertEnabledState(true, true, true);
+
+			FormTester formTester = tester.newFormTester("outerForm");
+			var message = assertThrows(WicketRuntimeException.class,
+				() -> formTester.submit("submit")).getMessage();
+			assertThat(message).contains("is not visible");
+		}
+
+		@Test
+		void doNotSubmitInvisibleButtons_innerForm()
+		{
+			assertEnabledState(true, true, true);
+
+			FormTester formTester = tester.newFormTester("outerForm:middleForm:innerForm");
+			var message = assertThrows(WicketRuntimeException.class,
+				() -> formTester.submit("submit")).getMessage();
+			assertThat(message).contains("is not visible");
+		}
+
+	}
+
+	@Nested
+	class WhenSubmitterDisabledFormSubmitTest
+	{
+
+		@BeforeEach
+		void before()
+		{
+			tester.startPage(new NestedFormsPage(true, false));
+		}
+
+		@Test
+		void doNotSubmitInvisibleButtons_middleForm()
+		{
+			assertEnabledState(true, true, true);
+
+			FormTester formTester = tester.newFormTester("outerForm:middleForm");
+
+			var message = assertThrows(WicketRuntimeException.class,
+				() -> formTester.submit("submit")).getMessage();
+			assertThat(message).contains("is not enabled");
+		}
+
+		@Test
+		void doNotSubmitInvisibleButtons_outerForm()
+		{
+			assertEnabledState(true, true, true);
+
+			FormTester formTester = tester.newFormTester("outerForm");
+			var message = assertThrows(WicketRuntimeException.class,
+				() -> formTester.submit("submit")).getMessage();
+			assertThat(message).contains("is not enabled");
+		}
+
+		@Test
+		void doNotSubmitInvisibleButtons_innerForm()
+		{
+			assertEnabledState(true, true, true);
+
+			FormTester formTester = tester.newFormTester("outerForm:middleForm:innerForm");
+			var message = assertThrows(WicketRuntimeException.class,
+				() -> formTester.submit("submit")).getMessage();
+			assertThat(message).contains("is not enabled");
+		}
+
+	}
+
 	/**
-	 * 
 	 * @param isOuterFormEnabled
 	 * @param isMiddleFormEnabled
 	 * @param isInnerFormEnabled
 	 */
-	private void assertEnabledState(boolean isOuterFormEnabled, boolean isMiddleFormEnabled,
+	private void assertEnabledState(
+		boolean isOuterFormEnabled, boolean isMiddleFormEnabled,
 		boolean isInnerFormEnabled)
 	{
 		assertEquals(isOuterFormEnabled, outerForm.isEnabled());
@@ -288,12 +383,12 @@ private void assertEnabledState(boolean isOuterFormEnabled, boolean isMiddleForm
 	}
 
 	/**
-	 * 
 	 * @param isOuterFormOnErrorCalled
 	 * @param isMiddleFormOnErrorCalled
 	 * @param isInnerFormOnErrorCalled
 	 */
-	private void assertOnErrorCalled(boolean isOuterFormOnErrorCalled,
+	private void assertOnErrorCalled(
+		boolean isOuterFormOnErrorCalled,
 		boolean isMiddleFormOnErrorCalled, boolean isInnerFormOnErrorCalled)
 	{
 		assertEquals(isOuterFormOnErrorCalled, outerForm.onErrorCalled);
@@ -302,12 +397,12 @@ private void assertOnErrorCalled(boolean isOuterFormOnErrorCalled,
 	}
 
 	/**
-	 * 
 	 * @param isOuterFormOnSubmitCalled
 	 * @param isMiddleFormOnSubmitCalled
 	 * @param isInnerFormOnSubmitCalled
 	 */
-	private void assertOnSubmitCalled(boolean isOuterFormOnSubmitCalled,
+	private void assertOnSubmitCalled(
+		boolean isOuterFormOnSubmitCalled,
 		boolean isMiddleFormOnSubmitCalled, boolean isInnerFormOnSubmitCalled)
 	{
 		assertEquals(isOuterFormOnSubmitCalled, outerForm.onSubmitCalled);
@@ -317,12 +412,13 @@ private void assertOnSubmitCalled(boolean isOuterFormOnSubmitCalled,
 
 	/**
 	 * @param formToBeSubmitted
-	 *            absolute path of the form to be submitted
+	 * 	absolute path of the form to be submitted
 	 * @param componentToGetError
-	 *            relative path to <code>formToBeSumitted</code> of the component to be changed
+	 * 	relative path to <code>formToBeSumitted</code> of the component to be changed
 	 * @return a {@link FormTester} instance
 	 */
-	private FormTester causeValidationErrorAndSubmit(String formToBeSubmitted,
+	private FormTester causeValidationErrorAndSubmit(
+		String formToBeSubmitted,
 		String componentToGetError)
 	{
 		FormTester formTester;

wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/FormTest.java

@@ -71,7 +71,8 @@ void defaultButton() throws Exception
 	@Test
 	void defaultButtonHierarchy() throws Exception
 	{
-		executeTest(FormHierarchyDefaultButtonTestPage.class, "FormHierarchyDefaultButtonTestPage_expected.html");
+		executeTest(FormHierarchyDefaultButtonTestPage.class,
+			"FormHierarchyDefaultButtonTestPage_expected.html");
 	}
 
 	/**
@@ -81,15 +82,17 @@ void defaultButtonHierarchy() throws Exception
 	void formMethods() throws Exception
 	{
 		executeTest(FormMethodTestPage.class, "FormMethodTestPage_expected.html");
-		
+
 		FormMethodTestPage page = (FormMethodTestPage)tester.getLastRenderedPage();
-	
+
 		// form action contains path and query 
-		assertEquals("var f = document.getElementById('formpost1');f.action='path?param1&param2=val%20ue';Wicket.Event.fire(f, 'submit');",
+		assertEquals(
+			"var f = document.getElementById('formpost1');f.action='path?param1&param2=val%20ue';Wicket.Event.fire(f, 'submit');",
 			page.postForm.getJsForListenerUrl("path?param1&param2=val%20ue").toString());
-		
+
 		// form action must not contain query (since ignored by browser), put into hidden form parameters instead 
-		assertEquals("document.getElementById('formget2_hf_0').innerHTML = '<input type=\"hidden\" name=\"param1\" value=\"\" /><input type=\"hidden\" name=\"param2\" value=\"val ue\" />';var f = document.getElementById('formget2');f.action='path';Wicket.Event.fire(f, 'submit');",
+		assertEquals(
+			"document.getElementById('formget2_hf_0').innerHTML = '<input type=\"hidden\" name=\"param1\" value=\"\" /><input type=\"hidden\" name=\"param2\" value=\"val ue\" />';var f = document.getElementById('formget2');f.action='path';Wicket.Event.fire(f, 'submit');",
 			page.getForm.getJsForListenerUrl("path?param1&param2=val%20ue").toString());
 	}
 
@@ -149,7 +152,8 @@ protected void onError()
 			}
 
 			@Override
-			public IResourceStream getMarkupResourceStream(final MarkupContainer container,
+			public IResourceStream getMarkupResourceStream(
+				final MarkupContainer container,
 				Class<?> containerClass)
 			{
 				return new StringResourceStream("<form wicket:id='form'></form>");
@@ -170,7 +174,7 @@ public IResourceStream getMarkupResourceStream(final MarkupContainer container,
 		assertTrue(page.error);
 	}
 
-	/** */
+	/**  */
 	public static class TestPage extends MockPageParametersAware
 	{
 		private static final long serialVersionUID = 1L;

wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/NestedFormsPage.java

@@ -45,18 +45,18 @@ public class NestedFormsPage extends WebPage
 	/**
 	 * Construct.
 	 */
-	public NestedFormsPage()
+	public NestedFormsPage(boolean submitterVisible, boolean submitterEnabled)
 	{
 		feedback = new FeedbackPanel("feedback");
 		add(feedback.setOutputMarkupId(true));
 
-		Form<?> outerForm = new NestableForm("outerForm");
+		Form<?> outerForm = new NestableForm("outerForm", submitterVisible, submitterEnabled);
 		add(outerForm.setOutputMarkupId(true));
 
-		Form<?> middleForm = new NestableForm("middleForm");
+		Form<?> middleForm = new NestableForm("middleForm", submitterVisible, submitterEnabled);
 		outerForm.add(middleForm.setOutputMarkupId(true));
 
-		Form<?> innerForm = new NestableForm("innerForm");
+		Form<?> innerForm = new NestableForm("innerForm", submitterVisible, submitterEnabled);
 		middleForm.add(innerForm.setOutputMarkupId(true));
 	}
 
@@ -79,11 +79,11 @@ public class NestableForm extends Form<NestableForm>
 
 		/**
 		 * Construct.
-		 * 
+		 *
 		 * @param id
 		 *            the form's id
 		 */
-        NestableForm(String id)
+		NestableForm(String id, boolean submitterVisible, boolean submitterEnabled)
 		{
 			super(id);
 			setDefaultModel(new CompoundPropertyModel<NestableForm>(this));
@@ -111,7 +111,7 @@ protected void onError(AjaxRequestTarget target)
 				}
 			});
 			add(new ToggleLink("toggle", this));
-			add(new SubmitLink("submit"));
+			add(new SubmitLink("submit").setVisible(submitterVisible).setEnabled(submitterEnabled));
 		}
 
 		@Override

wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java

@@ -801,25 +801,28 @@ public final void onFormSubmitted(IFormSubmitter submitter)
 			if (submitter == null)
 			{
 				submitter = findSubmitter();
+			}
 
-				if (submitter instanceof IFormSubmittingComponent)
-				{
-					IFormSubmittingComponent submittingComponent = (IFormSubmittingComponent)submitter;
-					Component component = (Component)submitter;
+			if (submitter instanceof IFormSubmittingComponent formSubmittingComponent)
+			{
+				Component component = (Component)submitter;
 
-					if (!component.isVisibleInHierarchy())
-					{
-						throw new WicketRuntimeException("Submit Button " +
-							submittingComponent.getInputName() + " (path=" +
-							component.getPageRelativePath() + ") is not visible");
-					}
+				if (!component.isVisibleInHierarchy())
+				{
+					throw new WicketRuntimeException("Submit Button " +
+						formSubmittingComponent.getInputName() +
+						" (path=" +
+						component.getPageRelativePath() +
+						") is not visible");
+				}
 
-					if (!component.isEnabledInHierarchy())
-					{
-						throw new WicketRuntimeException("Submit Button " +
-							submittingComponent.getInputName() + " (path=" +
-							component.getPageRelativePath() + ") is not enabled");
-					}
+				if (!component.isEnabledInHierarchy())
+				{
+					throw new WicketRuntimeException("Submit Button " +
+						formSubmittingComponent.getInputName() +
+						" (path=" +
+						component.getPageRelativePath() +
+						") is not enabled");
 				}
 			}