On Tue, Feb 17, 2009 at 10:02:56AM -0800, Igor Vaynberg wrote: > please open a jira issue and attach your patch there
Done, as http://issues.apache.org/jira/browse/WICKET-2111 Thanks, \Berry > > -igor > > On Tue, Feb 17, 2009 at 6:19 AM, (Berry) A.W. van Halderen > <[email protected]> wrote: > > L.S., > > > > We're using wicket heavily here with quite satisfaction. However in one of > > our attempts to setup integrated testing we found that one particular piece > > of > > wicket code isn't quite extendible enough for our needs, which is the > > generation of markup ids by the wicket session class. > > > > Since we think that the ability to extend this functionality is not limited > > to > > our use case, I'd like to propose a small change. The patch for this is at > > the end of this mail. > > > > The issue is the following; when a Component has no explicit markup-id set, > > the markup id is generated by the Session which has an internal counter and > > uses an increment of this to generate a mark-id. The flaw IMHO is that a > > Component requests the Session to generate an id, without passing it any > > context. Especially the most logical context, i.e. "please session, > > generate > > a markup id for _ME_" is missing. Therefore I'd propose that the > > Session.getMarkupId() is passes the Component object for which the markup id > > is to be generated. > > > > By default, the operation should remain as is and the Session object falls > > back to the default getMarkupId() without parameters, which is already > > overrideable. But now you can override the getMarkupId() and generate more > > useful markup ids. > > > > In our case, we are able to generate markup ids which contain part of the > > hierarchy and in this manner generate stable Ids, namely those which do not > > change after several requests. This particular usage may just work for our > > case (one page application, no back-button support, etc), but the > > fundamental > > overrideable method to generate more useful IDs is more widely applicable, > > hence my change request. > > > > If you look at the patch, its a little bit more than just the new method, > > that's because the patch is fashioned in such a way that the change is 100% > > compatible with previous workings of wicket in casu markup ids. Also, the > > getMarkupId used to return just an integer, while a string as return value > > is > > a more useful return value in a more context aware environment. To be sure > > the behavior is completely non-intrusive and compatible there is a little > > bit > > of glue coded needed. > > > > I can go into the details of the patch more, but this mail is too long as it > > is. > > > > Hope you accept this change, > > With kind regards, > > Berry van Halderen > > > > --- cut here --- > > > > Index: jdk-1.4/wicket/src/main/java/org/apache/wicket/Session.java > > =================================================================== > > *** jdk-1.4/wicket/src/main/java/org/apache/wicket/Session.java (revision > > 724306) > > --- jdk-1.4/wicket/src/main/java/org/apache/wicket/Session.java (working > > copy) > > *************** > > *** 1475,1478 **** > > --- 1475,1489 ---- > > { > > return sequence++; > > } > > + > > + /** > > + * Retrieves the next available session-unique value for the > > supplied Component > > + * > > + * @param component > > + * the component which requests the generation of a > > markup identifier > > + * @return session-unique value > > + */ > > + public Object getMarkupId(Component component) { > > + return new Integer(nextSequenceValue()); > > + } > > } > > Index: jdk-1.4/wicket/src/main/java/org/apache/wicket/Component.java > > =================================================================== > > *** jdk-1.4/wicket/src/main/java/org/apache/wicket/Component.java > > (revision 724306) > > --- jdk-1.4/wicket/src/main/java/org/apache/wicket/Component.java > > (working copy) > > *************** > > *** 1426,1437 **** > > return null; > > } > > > > ! final int generatedMarkupId = storedMarkupId instanceof > > Integer > > ! ? ((Integer)storedMarkupId).intValue() : > > Session.get().nextSequenceValue(); > > ! > > ! if (storedMarkupId == null) > > ! { > > ! setMarkupIdImpl(new Integer(generatedMarkupId)); > > } > > > > // try to read from markup > > --- 1426,1445 ---- > > return null; > > } > > > > ! String markupIdPostfix; > > ! if (!(storedMarkupId instanceof Integer)) { > > ! Object markupIdFromSession = > > Session.get().getMarkupId(this); > > ! if (storedMarkupId == null && markupIdFromSession > > != null) { > > ! setMarkupIdImpl(markupIdFromSession); > > ! } > > ! storedMarkupId = markupIdFromSession; > > ! } > > ! if (storedMarkupId instanceof Integer) { > > ! markupIdPostfix = Integer.toHexString(((Integer) > > storedMarkupId).intValue()).toLowerCase(); > > ! } else if (storedMarkupId instanceof String) { > > ! return (String) storedMarkupId; > > ! } else { > > ! markupIdPostfix = storedMarkupId.toString(); > > } > > > > // try to read from markup > > *************** > > *** 1449,1455 **** > > markupIdPrefix = getId(); > > } > > > > - String markupIdPostfix = > > Integer.toHexString(generatedMarkupId).toLowerCase(); > > markupIdPostfix = > > RequestContext.get().encodeMarkupId(markupIdPostfix); > > > > String markupId = markupIdPrefix + markupIdPostfix; > > --- 1457,1462 ---- > > > > --- cut here --- > > > > -- > > Berry A.W. van Halderen [email protected] / > > [email protected] > > Disclaimer: the above is the author's personal opinion and is not the > > opinion > > or policy of his employer or of the little green men that have been > > following > > him all day. > > -- Berry A.W. van Halderen [email protected] / [email protected] Disclaimer: the above is the author's personal opinion and is not the opinion or policy of his employer or of the little green men that have been following him all day.
