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

[4.0] Fix dispatcher to use the backend #18679

Closed
wants to merge 1 commit into from

Conversation

wilsonge
Copy link
Contributor

Pull Request for Issue #18626 .

Summary of Changes

Attempts to fix versioning in the frontend. I'm very unsure about the quality of this fix. But it does work

Testing Instructions

Checking versioning in the frontend editing view works

Expected result

It works

Actual result

Fatal Error

@ghost
Copy link

ghost commented Nov 18, 2017

I have tested this item 🔴 unsuccessfully on 4cd835a

Firefox: Modal Window open and closed.
Chrome: cmd+Click on "Versions" open Modal which stay open but Buttons (Rstore … Delete) don't work after Click on them.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18679.

@wilsonge wilsonge force-pushed the fix-frontend-versions branch from 4cd835a to 81d2407 Compare November 18, 2017 21:34
@wilsonge wilsonge changed the title Fix dispatcher to use the backend [4.0] Fix dispatcher to use the backend Nov 19, 2017
@mbabker
Copy link
Contributor

mbabker commented Nov 19, 2017

This might work for history but it isn't portable. The way getController is called by default it makes all of the existing config override logic just not work and I don't think we need to be overloading getController to repeat logic we're already running elsewhere (or move that logic into extended methods).

@wilsonge
Copy link
Contributor Author

It depends what we want to do with the Application object. It's not exactly the most sustainable thing to create an admin app instance i guess. There are various ways to make this more pretty from a method overriding perspective

@wilsonge wilsonge closed this Nov 19, 2017
@wilsonge wilsonge reopened this Nov 19, 2017
@mbabker
Copy link
Contributor

mbabker commented Nov 19, 2017

AFAIK you don't need to have an admin app instance to make it work right (we aren't doing that now in the proxies). The issue we have is getController is called without passing a client argument by default so it's resolving to the active app to generate namespaces, and in these proxy cases we need to explicitly state admin client to get the namespaces right.

@wilsonge
Copy link
Contributor Author

It's not just getController - it's also in the MVC factories

@mbabker
Copy link
Contributor

mbabker commented Nov 19, 2017

Same root issue. Without explicit client it resolves to active app. Honestly it's a higher level problem than this because there's so much dependent on active app and we shouldn't have to inject another one to make things work.

@wilsonge
Copy link
Contributor Author

Which is why i said earlier

It depends what we want to do with the Application object

@brianteeman
Copy link
Contributor

closed see #21208

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

Successfully merging this pull request may close these issues.

4 participants