You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by pa...@apache.org on 2020/02/03 20:42:51 UTC

[wicket] branch master updated: WICKET-6732: move onclick handlers to event binding for links

This is an automated email from the ASF dual-hosted git repository.

papegaaij pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/wicket.git


The following commit(s) were added to refs/heads/master by this push:
     new bcda1de  WICKET-6732: move onclick handlers to event binding for links
bcda1de is described below

commit bcda1de49a4b3faa74d0a11e893bba9d099ea9bc
Author: Emond Papegaaij <em...@topicus.nl>
AuthorDate: Mon Feb 3 21:38:45 2020 +0100

    WICKET-6732: move onclick handlers to event binding for links
---
 .../src/main/java/org/apache/wicket/Component.java |   4 +-
 .../wicket/ajax/markup/html/AjaxFallbackLink.java  |  12 ++-
 .../wicket/markup/html/link/ExternalLink.java      | 110 +++++++++++++--------
 .../org/apache/wicket/markup/html/link/Link.java   |  97 +++++++++++-------
 .../markup/html/link/BookmarkablePageLinkTest.java |  18 +++-
 5 files changed, 152 insertions(+), 89 deletions(-)

diff --git a/wicket-core/src/main/java/org/apache/wicket/Component.java b/wicket-core/src/main/java/org/apache/wicket/Component.java
index f3b31f7..fb5dc22 100644
--- a/wicket-core/src/main/java/org/apache/wicket/Component.java
+++ b/wicket-core/src/main/java/org/apache/wicket/Component.java
@@ -1384,11 +1384,13 @@ public abstract class Component
 	}
 
 	/**
+	 * THIS METHOD IS NOT PART OF THE WICKET PUBLIC API. DO NOT USE IT!
+	 * 
 	 * Get the first component tag in the associated markup
 	 * 
 	 * @return first component tag
 	 */
-	private ComponentTag getMarkupTag()
+	protected final ComponentTag getMarkupTag()
 	{
 		IMarkupFragment markup = getMarkup();
 		if (markup != null)
diff --git a/wicket-core/src/main/java/org/apache/wicket/ajax/markup/html/AjaxFallbackLink.java b/wicket-core/src/main/java/org/apache/wicket/ajax/markup/html/AjaxFallbackLink.java
index 08d0c33..7cb81cb 100644
--- a/wicket-core/src/main/java/org/apache/wicket/ajax/markup/html/AjaxFallbackLink.java
+++ b/wicket-core/src/main/java/org/apache/wicket/ajax/markup/html/AjaxFallbackLink.java
@@ -132,7 +132,7 @@ public abstract class AjaxFallbackLink<T> extends Link<T>
 	public abstract void onClick(final Optional<AjaxRequestTarget> target);
 
 	/**
-	 * Removes any inline 'onclick' attributes set by Link#onComponentTag(ComponentTag).
+	 * Checks if the tag supports href: a, area or link.
 	 * 
 	 * @param tag
 	 *            the component tag
@@ -142,9 +142,6 @@ public abstract class AjaxFallbackLink<T> extends Link<T>
 	{
 		super.onComponentTag(tag);
 
-		// Ajax links work with JavaScript Event registration
-		tag.remove("onclick");
-
 		String tagName = tag.getName();
 		if (isEnabledInHierarchy() &&
 			!("a".equalsIgnoreCase(tagName) || "area".equalsIgnoreCase(tagName) || "link".equalsIgnoreCase(tagName)))
@@ -157,4 +154,11 @@ public abstract class AjaxFallbackLink<T> extends Link<T>
 			findMarkupStream().throwMarkupException(msg);
 		}
 	}
+
+	@Override
+	protected boolean useJSEventBindingWhenNeeded()
+	{
+		// AjaxFallbackLink uses Ajax event binding.
+		return false;
+	}
 }
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/ExternalLink.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/ExternalLink.java
index e37799b..ce414d4 100644
--- a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/ExternalLink.java
+++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/ExternalLink.java
@@ -17,6 +17,8 @@
 package org.apache.wicket.markup.html.link;
 
 import org.apache.wicket.markup.ComponentTag;
+import org.apache.wicket.markup.head.IHeaderResponse;
+import org.apache.wicket.markup.head.OnEventHeaderItem;
 import org.apache.wicket.model.IModel;
 import org.apache.wicket.model.Model;
 import org.apache.wicket.protocol.http.WebApplication;
@@ -159,54 +161,76 @@ public class ExternalLink extends AbstractLink
 		}
 		else if (getDefaultModel() != null)
 		{
-			Object hrefValue = getDefaultModelObject();
-			if (hrefValue != null)
+			String url = renderUrl();
+			// if the tag is an anchor proper
+			if (url != null
+				&& (tag.getName().equalsIgnoreCase("a") || tag.getName().equalsIgnoreCase("link")
+					|| tag.getName().equalsIgnoreCase("area")))
 			{
-				String url = hrefValue.toString();
-
-				if (contextRelative)
-				{
-					if (url.length() > 0 && url.charAt(0) == '/')
-					{
-						url = url.substring(1);
-					}
-					url = UrlUtils.rewriteToContextRelative(url, RequestCycle.get());
-				}
-
-				// if the tag is an anchor proper
-				if (tag.getName().equalsIgnoreCase("a") || tag.getName().equalsIgnoreCase("link") ||
-					tag.getName().equalsIgnoreCase("area"))
-				{
-					// generate the href attribute
-					tag.put("href", url);
-
-					// Add any popup script
-					if (popupSettings != null)
-					{
-						// NOTE: don't encode to HTML as that is not valid
-						// JavaScript
-						tag.put("onclick", popupSettings.getPopupJavaScript());
-					}
-				}
-				else
-				{
-					// generate a popup script by asking popup settings for one
-					if (popupSettings != null)
-					{
-						popupSettings.setTarget("'" + url + "'");
-						String popupScript = popupSettings.getPopupJavaScript();
-						tag.put("onclick", popupScript);
-					}
-					else
-					{
-						// or generate an onclick JS handler directly
-						tag.put("onclick", "window.location.href='" + url + "';return false;");
-					}
-				}
+				// generate the href attribute
+				tag.put("href", url);
 			}
 		}
 	}
 
+	@Override
+	public void renderHead(IHeaderResponse response)
+	{
+		super.renderHead(response);
+
+		String url = renderUrl();
+		if (isEnabledInHierarchy() && url != null)
+		{
+			if (popupSettings != null)
+			{
+				popupSettings.setTarget("'" + url + "'");
+				response.render(OnEventHeaderItem.forComponent(this, "click",
+					popupSettings.getPopupJavaScript()));
+				return;
+			}
+
+			ComponentTag tag = getMarkupTag();
+			// finally, when the tag is not a normal link
+			if (!(tag.getName().equalsIgnoreCase("a") || tag.getName().equalsIgnoreCase("link")
+				|| tag.getName().equalsIgnoreCase("area")
+				|| tag.getName().equalsIgnoreCase("script")
+				|| tag.getName().equalsIgnoreCase("style")))
+			{
+				// generate an onclick JS handler directly
+				// in firefox when the element is quickly clicked 3 times a second request is
+				// generated during page load. This check ensures that the click is ignored
+				response.render(OnEventHeaderItem.forComponent(this, "click",
+					"var win = this.ownerDocument.defaultView || this.ownerDocument.parentWindow; "
+						+ "if (win == window) { window.location.href='" + url
+						+ "'; } ;return false"));
+				return;
+			}
+		}
+	}
+
+	/**
+	 * @return the URL for this link
+	 */
+	private String renderUrl()
+	{
+		Object hrefValue = getDefaultModelObject();
+		if (hrefValue == null)
+		{
+			return null;
+		}
+
+		String url = hrefValue.toString();
+		if (contextRelative)
+		{
+			if (url.length() > 0 && url.charAt(0) == '/')
+			{
+				url = url.substring(1);
+			}
+			url = UrlUtils.rewriteToContextRelative(url, RequestCycle.get());
+		}
+		return url;
+	}
+
 	/**
 	 * @return True if this link is automatically prepended with ../ to make it relative to the
 	 *         context root.
diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/Link.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/Link.java
index b7b303c..305f1ed 100644
--- a/wicket-core/src/main/java/org/apache/wicket/markup/html/link/Link.java
+++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/link/Link.java
@@ -22,6 +22,8 @@ import org.apache.wicket.IRequestListener;
 import org.apache.wicket.Page;
 import org.apache.wicket.WicketRuntimeException;
 import org.apache.wicket.markup.ComponentTag;
+import org.apache.wicket.markup.head.IHeaderResponse;
+import org.apache.wicket.markup.head.OnEventHeaderItem;
 import org.apache.wicket.model.IModel;
 import org.apache.wicket.request.mapper.parameter.PageParameters;
 
@@ -29,7 +31,7 @@ import org.apache.wicket.request.mapper.parameter.PageParameters;
  * Implementation of a hyperlink component. A link can be used with an anchor (&lt;a href...)
  * element or any element that supports the onclick javascript event handler (such as buttons, td
  * elements, etc). When used with an anchor, a href attribute will be generated. When used with any
- * other element, an onclick javascript event handler attribute will be generated.
+ * other element, a click javascript event handler will be added.
  * <p>
  * You can use a link like:
  * 
@@ -366,54 +368,77 @@ public abstract class Link<T> extends AbstractLink implements IRequestListener,
 			{
 				// generate the href attribute
 				tag.put("href", url);
-
-				// Add any popup script
-				if (popupSettings != null)
-				{
-					// NOTE: don't encode to HTML as that is not valid
-					// JavaScript
-					tag.put("onclick", popupSettings.getPopupJavaScript());
-				}
 			}
 			else if (tag.getName().equalsIgnoreCase("script") ||
 				tag.getName().equalsIgnoreCase("style"))
 			{
 				tag.put("src", url);
 			}
-			else
-			{
-				// generate a popup script by asking popup settings for one
-				if (popupSettings != null)
-				{
-					popupSettings.setTarget("'" + url + "'");
-					String popupScript = popupSettings.getPopupJavaScript();
-					tag.put("onclick", popupScript);
-				}
-				else
-				{
-					// or generate an onclick JS handler directly
-					// in firefox when the element is quickly clicked 3 times a second request is
-					// generated during page load. This check ensures that the click is ignored
-					tag.put(
-						"onclick",
-						"var win = this.ownerDocument.defaultView || this.ownerDocument.parentWindow; " +
-							"if (win == window) { window.location.href='" +
-							url +
-							"'; } ;return false");
-				}
-			}
+		}
+		else
+		{
+			disableLink(tag);
+		}
+	}
+	
+	@Override
+	public void renderHead(IHeaderResponse response)
+	{
+		super.renderHead(response);
+		// If we're disabled
+		if (isEnabledInHierarchy() && useJSEventBindingWhenNeeded())
+		{
+			ComponentTag tag = getMarkupTag();
 
+			// Set href to link to this link's linkClicked method
+			CharSequence url = getURL();
+
+			// append any anchor
+			url = appendAnchor(tag, url);
 
 			// If the subclass specified javascript, use that
 			final CharSequence onClickJavaScript = getOnClickScript(url);
 			if (onClickJavaScript != null)
 			{
-				tag.put("onclick", onClickJavaScript);
+				response.render(OnEventHeaderItem.forComponent(this, "click", onClickJavaScript));
+				return;
+			}
+
+			// next check for popup settings
+			if (popupSettings != null)
+			{
+				popupSettings.setTarget("'" + url + "'");
+				response.render(OnEventHeaderItem.forComponent(this, "click",
+					popupSettings.getPopupJavaScript()));
+				return;
+			}
+
+			// finally, when the tag is not a normal link
+			if (!(tag.getName().equalsIgnoreCase("a") || tag.getName().equalsIgnoreCase("link")
+				|| tag.getName().equalsIgnoreCase("area")
+				|| tag.getName().equalsIgnoreCase("script")
+				|| tag.getName().equalsIgnoreCase("style")))
+			{
+				// generate an onclick JS handler directly
+				// in firefox when the element is quickly clicked 3 times a second request is
+				// generated during page load. This check ensures that the click is ignored
+				response.render(OnEventHeaderItem.forComponent(this, "click",
+					"var win = this.ownerDocument.defaultView || this.ownerDocument.parentWindow; "
+						+ "if (win == window) { window.location.href='" + url
+						+ "'; } ;return false"));
+				return;
 			}
 		}
-		else
-		{
-			disableLink(tag);
-		}
+	}
+	
+	/**
+	 * This method can be overridden by a subclass to disable the JS event binding or provide custom
+	 * event binding code is used.
+	 * 
+	 * @return true when a javascripot event binding must used to handle the click event.
+	 */
+	protected boolean useJSEventBindingWhenNeeded()
+	{
+		return true;
 	}
 }
\ No newline at end of file
diff --git a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/BookmarkablePageLinkTest.java b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/BookmarkablePageLinkTest.java
index 4988479..4f0835d 100644
--- a/wicket-core/src/test/java/org/apache/wicket/markup/html/link/BookmarkablePageLinkTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/markup/html/link/BookmarkablePageLinkTest.java
@@ -75,14 +75,22 @@ class BookmarkablePageLinkTest extends WicketTestCase
 	@Test
 	void customParametersWithSpecialCharacters()
 	{
-		BookmarkablePageLink<MockPageWithLink> link = new BookmarkablePageLink<MockPageWithLink>(
-			"link", MockPageWithLink.class);
+		BookmarkablePageLink<MockPageWithLink> link =
+			new BookmarkablePageLink<MockPageWithLink>("link", MockPageWithLink.class);
 		link.getPageParameters().set("urlEscapeNeeded", "someone's ^b%a&d pa\"rameter");
 
 		tester.startComponentInPage(link, null);
+		String expected =
+			"<html><head><script type=\"text/javascript\" src=\"./resource/org.apache.wicket.resource.JQueryResourceReference/jquery/jquery-3.4.1.js\"></script>\n"
+				+ "<script type=\"text/javascript\" src=\"./resource/org.apache.wicket.ajax.AbstractDefaultAjaxBehavior/res/js/wicket-ajax-jquery.js\"></script>\n"
+				+ "<script type=\"text/javascript\">\n" + "/*<![CDATA[*/\n"
+				+ "Wicket.Event.add(window, \"domready\", function(event) { \n"
+				+ "Wicket.Event.add('link1', 'click', function(event) { var win = this.ownerDocument.defaultView || this.ownerDocument.parentWindow; if (win == window) { window.location.href='./bookmarkable/org.apache.wicket.MockPageWithLink?urlEscapeNeeded=someone%27s+%5Eb%25a%26d+pa%22rameter'; } ;return false;});;\n"
+				+ "Wicket.Event.publish(Wicket.Event.Topic.AJAX_HANDLERS_BOUND);\n" + ";});\n"
+				+ "/*]]>*/\n" + "</script>\n"
+				+ "</head><body><span wicket:id=\"link\" id=\"link1\"></span></body></html>";
+
 		String response = tester.getLastResponse().getDocument();
-		assertEquals(
-			"<html><body><span wicket:id=\"link\" onclick=\"var win = this.ownerDocument.defaultView || this.ownerDocument.parentWindow; if (win == window) { window.location.href=&#039;./bookmarkable/org.apache.wicket.MockPageWithLink?urlEscapeNeeded=someone%27s+%5Eb%25a%26d+pa%22rameter&#039;; } ;return false\"></span></body></html>",
-			response);
+		assertEquals(expected, response);
 	}
 }