please open a jira issue and attach your patch there -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. >
