Thx Martin, I cleaned up the code. I found one more issue, our bookmarkable mapper isn't testing if the page is bookmarkable or not at AbstractBookmarkableMapper#checkPageClass, which can make the mapper to create URLs that would be mapped to handlers that couldn't be resolved; already fixed it on the branch [1].
Also checked in some test expectations so you can have an idea of what will change [2]. There are more 50 exactly same expectation to be updated still :) Lets first make sure we are in the right path. 1 - https://github.com/apache/wicket/commit/1ec0b8b3637feb19da4f87b485e96bfc7ce4e992 2 - https://github.com/apache/wicket/commit/fba8bddd98da943ea74c9c04d90f17c75417c2fe Cheers, Pedro Santos On Wed, Jan 9, 2013 at 6:35 PM, Martin Grigorov <[email protected]>wrote: > I've commented in the commit thread. > > > On Wed, Jan 9, 2013 at 9:53 PM, Pedro Santos <[email protected]> wrote: > > > Nice, I moved the logic to AbstractBookmarkableMapper in this branch: > > sandbox/bookmarkable-callback-url > > > > Let me know if the changes are ok before I start to update our test cases > > expectations to assert the new encoded URL. > > > > Please do these additional changes in the same branch so we can see what > expectations have changed. > > > > > > Cheers, > > > > > > Pedro Santos > > > > > > On Sun, Jan 6, 2013 at 8:34 PM, Martin Grigorov <[email protected] > > >wrote: > > > > > Hi Pedro, > > > > > > I agree with you. > > > https://issues.apache.org/jira/browse/WICKET-4686 is similar - the > > support > > > for path parameters should be moved the same way too. > > > > > > > > > On Mon, Jan 7, 2013 at 12:05 AM, Pedro Santos <[email protected]> > > wrote: > > > > > > > Hi, > > > > > > > > we have the recreateMountedPagesAfterExpiry flag at page settings to > > tell > > > > mounted mapper to encode enough info at callback urls so the mounted > > page > > > > can be recreated after expired and the callback be invoked. > > > > > > > > Why this flag works only for mounted pages? It looks we are > segmenting > > > too > > > > much a functionality. I think that a more consistent way of to honor > > such > > > > flag is by improving AbstractBookmarkableMapper itself and not only > one > > > of > > > > its extension (MountedMapper). > > > > > > > > Let me know what you think so we can improve bookmarkable mapper. > > > > > > > > Cheers, > > > > > > > > Pedro Santos > > > > > > > > > > > > > > > > -- > > > Martin Grigorov > > > jWeekend > > > Training, Consulting, Development > > > http://jWeekend.com <http://jweekend.com/> > > > > > > > > > -- > Martin Grigorov > jWeekend > Training, Consulting, Development > http://jWeekend.com <http://jweekend.com/> >
