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

Subsystem layouts forget own location when the main error view is rendered #456

Closed
sneiland opened this Issue Oct 22, 2016 · 6 comments

Comments

Projects
None yet
2 participants
@sneiland

sneiland commented Oct 22, 2016

FW/1 3.5 using subsystems 2.0

When generating links in a subsystem layout the buildUrl function call buildurl("section.item") will normally correctly generate the link "index.cfm?action=subsystem:section.item".

However if an error occurs the buildurl function defaults to the subsystem of the error template, i.e. the link generated is "index.cfm?action=section.item".

@seancorfield

This comment has been minimized.

Show comment
Hide comment
@seancorfield

seancorfield Oct 24, 2016

Member

Can you add or publish a small self-contained example of exactly what you're seeing? Based on your description, I have a number of questions and it would be simpler to just look at the code.

Member

seancorfield commented Oct 24, 2016

Can you add or publish a small self-contained example of exactly what you're seeing? Based on your description, I have a number of questions and it would be simpler to just look at the code.

@seancorfield

This comment has been minimized.

Show comment
Hide comment
Member

seancorfield commented Nov 11, 2016

@sneiland

This comment has been minimized.

Show comment
Hide comment
@sneiland

sneiland Nov 20, 2016

Sorry Sean been otherwise occupied. I narrowed the cause of the error further. If you only have a default layout file in your subsystem then the error will be rendered in the main apps error viewfile without first rendering the subsystems layout.

If however you specify a different layout in your subsystem controller then that subsystem layout will be rendered with the error contents within it, however any navigation links in that layout will be made relative to the main application and not the subsystem as before.

I am including a test application that should demonstrate the error in the subsystems layout nav when you enter the test:main.breakit action.
test_error_case.zip

Sorry Sean been otherwise occupied. I narrowed the cause of the error further. If you only have a default layout file in your subsystem then the error will be rendered in the main apps error viewfile without first rendering the subsystems layout.

If however you specify a different layout in your subsystem controller then that subsystem layout will be rendered with the error contents within it, however any navigation links in that layout will be made relative to the main application and not the subsystem as before.

I am including a test application that should demonstrate the error in the subsystems layout nav when you enter the test:main.breakit action.
test_error_case.zip

@seancorfield

This comment has been minimized.

Show comment
Hide comment
@seancorfield

seancorfield Nov 25, 2016

Member

This is a somewhat subtle issue and is not actually related to error handling, per se, as far as I can tell.

When you set a layout, even if the layout is part of a subsystem, the layout will be evaluated in the context of the current request's subsystem. You can see this, for example, if you call setLayout() with test:otherlayout in the main controller: you get that layout but it is executed in the context of the main.default request, i.e., not the test subsystem.

When an error is raised, the request will execute in the context of the error handler request.

What should probably happen here is for the _fw1 structure to be cleaned up better so any override of the layout is reset (and then whatever the error handler does is what gets executed).

Member

seancorfield commented Nov 25, 2016

This is a somewhat subtle issue and is not actually related to error handling, per se, as far as I can tell.

When you set a layout, even if the layout is part of a subsystem, the layout will be evaluated in the context of the current request's subsystem. You can see this, for example, if you call setLayout() with test:otherlayout in the main controller: you get that layout but it is executed in the context of the main.default request, i.e., not the test subsystem.

When an error is raised, the request will execute in the context of the error handler request.

What should probably happen here is for the _fw1 structure to be cleaned up better so any override of the layout is reset (and then whatever the error handler does is what gets executed).

@seancorfield

This comment has been minimized.

Show comment
Hide comment
@seancorfield

seancorfield Nov 25, 2016

Member

I just tested, and having the onError() handler remove the overrideLayoutAction key does indeed solve this -- you get the default layout because that's what the main.error handler would implicitly use.

That leaves the question of whether layouts should be evaluated in the context of their own subsystem or the current request's subsystem (as happens now). I can see arguments for both but changing to the former could be a breaking change for some folks and I suspect deliberately setting a layout to pick up another subsystem's files is very much "caveat programmer".

Member

seancorfield commented Nov 25, 2016

I just tested, and having the onError() handler remove the overrideLayoutAction key does indeed solve this -- you get the default layout because that's what the main.error handler would implicitly use.

That leaves the question of whether layouts should be evaluated in the context of their own subsystem or the current request's subsystem (as happens now). I can see arguments for both but changing to the former could be a breaking change for some folks and I suspect deliberately setting a layout to pick up another subsystem's files is very much "caveat programmer".

@seancorfield seancorfield self-assigned this Nov 25, 2016

@seancorfield seancorfield added this to the 4.1 milestone Nov 25, 2016

@seancorfield

This comment has been minimized.

Show comment
Hide comment
@seancorfield

seancorfield Nov 25, 2016

Member

Implemented on develop -- feel free to try it out.

Member

seancorfield commented Nov 25, 2016

Implemented on develop -- feel free to try it out.

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