Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WATask does a redirect #121

Closed
GoogleCodeExporter opened this issue Mar 25, 2015 · 10 comments
Closed

WATask does a redirect #121

GoogleCodeExporter opened this issue Mar 25, 2015 · 10 comments

Comments

@GoogleCodeExporter
Copy link

See subject

Original issue reported on code.google.com by renggli on 29 Aug 2008 at 4:50

@GoogleCodeExporter
Copy link
Author

Name: Seaside-Core-lr.189
Author: lr
Time: 29 August 2008, 6:47:42 pm
UUID: a6af2867-e3c7-41eb-a6e8-2fbbafecedc2
Ancestors: Seaside-Core-lr.188

- yet another try at chaning how tasks works

Original comment by renggli on 29 Aug 2008 at 4:50

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Please provide feedback on Seaside-Core-lr.189. It appears to me that this 
solves the problem and does not 
break existing code. This also fixes Issue 16.

Original comment by renggli on 29 Aug 2008 at 4:59

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Seaside-Core-lr.189

Original comment by renggli on 30 Aug 2008 at 10:04

  • Changed state: Fixed
  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Name: Seaside-Component-lr.11
Author: lr
Time: 30 September 2008, 6:54:50 pm
UUID: 91dbace0-ee93-4849-8f92-7d7868a58dae
Ancestors: Seaside-Component-jf.10

- restored the old and crappy code of WATask that does a redirect, but that's 
the only thing that seems to 
work
- this re-opens Issue 16 and Issue 121
- this closes Issue 191

Original comment by renggli on 30 Sep 2008 at 4:55

  • Changed state: Accepted
  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

See also Issue 136 which should maybe be done before carrying on with this.

Original comment by [email protected] on 5 Oct 2008 at 12:33

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Original comment by [email protected] on 5 Oct 2008 at 12:33

  • Added labels: Priority-Low
  • Removed labels: Priority-Medium

@GoogleCodeExporter
Copy link
Author

I traced this a bit further tonight.

So Lukas was trying to change WATask so that it runs #go from within
#nextPresentersDo:. The problem with this is that the first time 
#nextPresentersDo:
is called for the task (at least in this case) is during #updateUrl: in a
WARedirectContinuation.

This means that the continuation is captured with that stack. His code also 
catches
and ignores WARenderNotifications, which means that when we come back into the
callback, do something else and then trigger a notification, the code carries 
on as
if it were in the middle of #updateUrl: from the earlier render continuation.

I tried monkeying around with WATask>>nextPresentersDo: and
WARedirectContinuation>>run to see if I could come up with a working scenario 
but
it's pretty ugly and I can't quite get it to work so far. With the methods
implemented as follows, the functional test sort of works (but the session and
continuation keys are in the form, not the URL, and we don't get a redirect 
between
action phases).

nextPresentersDo: aBlock
    self activeComponent == self ifTrue:
        [ self answer: self go ].
    super nextPresentersDo: aBlock

run
    | url |
    [url := self url.
    self respond: [ :response | response redirectTo: url ] ]
        on: WARenderNotification do: [ :n | self newRenderContinuation run]


But even if we figure out how to get this working it feels pretty fragile... 
#call:
works because we know that any time it could be called (ie. during the last 
callback
of an action phase), it is safe to totally abort what we are doing and start
something else. This just doesn't hold true in the general case of "any time we 
call
#nextPresentersDo:", I think.

Original comment by [email protected] on 7 Oct 2008 at 10:37

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Thank you for the detailled write-up. Your observations match my own 
observations.

I suggest to close this bug, trying to "fix it" isn't worth the trouble. Tasks 
are rarely used anyway and the problem can be easily avoided if really needed 
(by using a callback).

I will look at Issue 16 then, that's the one with the lightbox.

Original comment by renggli on 8 Oct 2008 at 6:10

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Yeah I don't think an extra redirect is the end of the world.

I can't help feeling that this might be solvable with some kind of "about to 
render",
"about to process" notifications that you *could* abort but I haven't really 
thought
it through.

It could be left open on low priority if you think we might want to try to come 
back
to it at some point. Otherwise, yeah, close it.

Original comment by [email protected] on 8 Oct 2008 at 8:13

  • Added labels: Type-CleanUp
  • Removed labels: Type-Defect

@GoogleCodeExporter
Copy link
Author

To be reopened later, if necessary.

Original comment by renggli on 8 Oct 2008 at 8:28

  • Changed state: WontFix
  • Added labels: ****
  • Removed labels: ****

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant