18 May
2011
18 May
'11
11:15 p.m.
From Julian:
I'm sure if this is relevant anymore but it's what I was writing
before we talked. Feel free to forward to the list if you think it's
relevant/helpful/interesting/whatever.
Julian
---------------
I'm not sure it's a good fix in the general case. If you don't have a
continuation key, you shouldn't (by definition) have any existing
(backtracked) state. Relying on the root presenter currently in the
Session is problematic since that value is backtracked for each
request; you'll simply get whichever root happened to be used by
whatever request was last handled by that session. If you don't happen
to be ever changing the root, then it may well be what you want, but
it's not right in the general case.
Incidentally, this also touches on the flaw I was discussing the other
week with our snapshots: because we have no continuation key, we
really ought to nil out any backtracked data before processing the
request (at least that way we have a consistent state while handling
the request). We don't run into the problem with the root presenter
because #start reinitializes it, but for other backtracked state,
you'd just get random stuff in there based on the last-handled
request.
A more appropriate general fix (at least in this case) could be to
take the last-used continuation and apply its state before handling a
request with no _k. However, we don't always know the last-used
(depending on the caching mechanism in place) and I'm not convinced
that is actually always what people want. I still feel pretty
confident that the "right" semantics of an Application/Session request
with no _k is to start a new main/render-loop.
On 18 May 2011 15:33, Nick Ager <nick.ager(a)gmail.com> wrote:
Hi,
I've noticed a problem with cookies enabled and Pier. The problem arises
when you browse to a URL without the continuation key, which causes a new
instance of the application to be created ie PRPierFrame new. You can see
the issue with the following single page WACounter Pier app:
| rootPage |
rootPage := (PRPage named: 'rootPage') contents: '+counter+'; yourself.
rootPage localEnvironment: ((PRComponent named: 'contents')
componentClass: PRContentsWidget; yourself).
rootPage addChild: ((PRComponent named: 'counter') componentClass:
WACounter; yourself).
PRKernel reset.
(PRPierFrame registerAsApplication: 'pier' kernel: (PRKernel named:
'testKernel' root: rootPage)) preferenceAt: #useCookies put: true.
Browse to localhost:xxxx/pier you should see the familiar counter
component. Increment the counter a few times. Now open a new tab, browse
to localhost:xxxx/pier and decrement the counter a couple of times. Return
to the first tab and try incrementing again. The counter will increment from
the value of the second counter.
I tested the same code snippet in Pier 1 on Seaside 2.8 and the component
increments from the value on the page ie it behaves as you'd expect a
Seaside component to behave.
The problem appears to arise in:
WASession>>#handleFiltered: aRequestContext
| key continuation |
key := aRequestContext request fields
at: self actionField
ifAbsent: [ ^ self start ].
continuation := continuations
at: key
ifAbsent: [ ^ self unknownRequest ].
continuation value
if there is no continuation key, #start is called on the session, which
creates a new root component. I have a fix, though it's in such a core part
of Seaside, I'm hesitant to suggest it. If I change WARenderLoopMain>>#start
from:
start
| root |
root := self createRoot.
self session properties at: #presenter put: root.
self prepareRoot: root.
((self application preferenceAt: #renderPhaseContinuationClass) new)
captureAndInvoke
to:
start
| root |
root := self session presenter.
root ifNil: [
root := self createRoot.
self session properties at: #presenter put: root.
self prepareRoot: root ].
((self application preferenceAt: #renderPhaseContinuationClass) new)
captureAndInvoke
...the root component is recovered from the session (if present) and no
longer created.
Is this a valid fix?
Thanks
Nick