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

Fix content-type header to application/json #43689

Open
wants to merge 1 commit into
base: 4.4-dev
Choose a base branch
from

Conversation

rubenferreira97
Copy link

@rubenferreira97 rubenferreira97 commented Jun 21, 2024

Pull Request for Issue #43641.

Summary of Changes

JsonDocument in Joomla 4.4.5 wrongly sets text/plain to all browsers (tested on Edge, Firefox and Chrome). Browsers do not usually send application/json, instead they send */* with some lower priority.

Note: Joomla 5 already fixed this, so this change needs to be merged to Joomla 4 (maybe Joomla 4.4.6?).

Testing Instructions

Ping and API endpoint that extends JsonDocument. In my case I am returning a normal json response instead of a json:api.

Actual result BEFORE applying this Pull Request

Ping an API endpoint like localhost/api/index.php/v1/example:
Content-Type: text/plain; charset=utf-8

Expected result AFTER applying this Pull Request

Ping an API endpoint like localhost/api/index.php/v1/example:
Content-Type: application/json; charset=utf-8

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@rubenferreira97 rubenferreira97 changed the title Fix content-type header to `application/json Fix content-type header to application/json Jun 21, 2024
@laoneo
Copy link
Member

laoneo commented Jun 21, 2024

Can you reference the pr which fixed this issue already in 5. The change here is a BC break, so not sure if the need to fix it weight's higher than the BC policy.

@rubenferreira97
Copy link
Author

rubenferreira97 commented Jun 21, 2024

@laoneo The PR which fixed this issue was #39397. IMO this is a bug that should be fixed since JsonDocument should always return application/json by default. Even the IE9 fix was a hacky workaround. Don't know about Joomla BC policy, but are people really expecting this and working around this behavior?

@laoneo
Copy link
Member

laoneo commented Jun 21, 2024

Problem is that this behavior did exist now for a long time. Changing it in a patch release might cause unexpected issues in extensions using that class as it belongs to the public API. We will discuss it in maintainers group and report back.

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

Successfully merging this pull request may close these issues.

None yet

3 participants