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

3.x AuthComponent ajaxLayout ? #9894

Closed
dereuromark opened this issue Dec 20, 2016 · 8 comments
Closed

3.x AuthComponent ajaxLayout ? #9894

dereuromark opened this issue Dec 20, 2016 · 8 comments
Assignees
Labels
Milestone

Comments

@dereuromark
Copy link
Member

The 3.x code seems to have some dead code:

        if (!empty($this->_config['ajaxLogin'])) {
            $controller->viewBuilder()->templatePath('Element');
            $response = $controller->render(
                $this->_config['ajaxLogin'],
                $this->RequestHandler->ajaxLayout
            );

            return $response->withStatus(403);
        }

$this->RequestHandler->ajaxLayout does not seem to exist.

maybe we can safely remove it?

@dereuromark dereuromark added this to the 3.3.11 milestone Dec 20, 2016
@markstory markstory self-assigned this Dec 20, 2016
@markstory
Copy link
Member

I guess the automatic usage of AjaxView is masking this issue.

@dereuromark
Copy link
Member Author

dereuromark commented Dec 21, 2016

Also FormHelper::_isRequiredField() seems to be a dead method.
At least in 3.4 then.

@littleylv
Copy link
Contributor

littleylv commented Dec 22, 2016

I have opened #8854 about $this->RequestHandler->ajaxLayout once before.

@dereuromark
Copy link
Member Author

Time to get rid of this dead code then :)

@markstory
Copy link
Member

One concern I have with removing this 'dead' code is that it is possible that an application developer could be doing something like:

public function beforeFilter($event)
{
   $this->RequestHandler->ajaxLayout = 'custom_ajax';
}

Us removing the cited line would 'break' this application code. I recognize that we don't have test coverage for this behavior, but it could break userland code.

@dereuromark
Copy link
Member Author

With a minor 3.4 it could still be safe to communicate this as not supported.
But OK, maybe deprecating it and then cleaning it up for 4.0 is the safer option.

markstory added a commit that referenced this issue Dec 22, 2016
This is a pretty odd property, that we should deprecate now to avoid
keeping it around much longer.

Refs #9894
@markstory
Copy link
Member

Pull request up to mark property as deprecated.

@dereuromark
Copy link
Member Author

Also FormHelper::_isRequiredField() seems to be a dead method.
At least in 3.4 then.

wasnt yet discussed, though

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

No branches or pull requests

3 participants