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

Allow KCFinder to send back a json encoded response instead of string… #203

Merged
merged 1 commit into from Apr 27, 2018

Conversation

seamuslee001
Copy link
Contributor

… javascript

ping @colemanw

if (!isset($js))
if ($this->outputFormat == 'json') {
$json = $this->callBack_json($url, $message);
echo json_encode($json);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should set a content type header here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@colemanw what would you recommend for content type?

header("Content-Type: text/html; charset={$this->charset}");
echo "<html><body>$js</body></html>";
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's something odd going on here. The deleted line on 762 has no opening bracket, but the line you aded on 778 does. I think that may be causing the odd behavior.

header("Content-Type: text/html; charset={$this->charset}");
echo "<html><body>$js</body></html>";
header("Content-Type: text/html; charset={$this->charset}");
echo "<html><body>$js</body></html>";
Copy link
Member

@colemanw colemanw Apr 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above 2 lines should not be inside that conditional. The original version of this file only has 1 line "inside" the conditional (the if statement doesn't have brackets).

… javascript

Add in Content-Type header as per Coleman
@seamuslee001
Copy link
Contributor Author

thanks @colemanw should be fixed now me thinks

@colemanw
Copy link
Member

This looks good now and is working correctly according to my testing.

@colemanw colemanw merged commit 7c27432 into civicrm:master Apr 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants