Skip to content

[HttpKernel][WebProfilerBundle] Getting the cached client mime type instead of guessing it again #29285

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

Merged
merged 1 commit into from
Nov 24, 2018

Conversation

yceruto
Copy link
Member

@yceruto yceruto commented Nov 22, 2018

Q A
Branch? master (4.2)
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #29284
License MIT
Doc PR -

The 1st commit fixes the bug based on the current implementation. But I'd like to improve this new feature a little bit with the 2nd commit:

  • Renaming "Uploaded files" title by "FILES Parameters" and (sub-section of POST parameters) changing the table using key/value structure (being consistent with other sections).
  • The above allow us also to show nested paramaters. (It is useful to know where this file(s) comes from)
  • And show all info about the UploadedFile object. (It might be useful too)

files-parameters

@ro0NL
Copy link
Contributor

ro0NL commented Nov 22, 2018

👍 for the new approach.

Im not sure about "FILES parameters", that's not a real HTTP verb. In HTTP context we're dealing "multipart form-data" (part of the POST params).

I suggest to keep the current title. At most it could be moved as h4 under POST parameters

@yceruto
Copy link
Member Author

yceruto commented Nov 22, 2018

@ro0NL I thought "FILES Parameters" is more according to $_FILES var as well as "POST Parameters" and $_POST | "GET Parameters" and $_GET. But I see your point.

@ro0NL
Copy link
Contributor

ro0NL commented Nov 22, 2018

Right, but im not sure we should use PHP terminology. It's also Server Parameters not SERVER 😅

@yceruto yceruto force-pushed the request_files_collector branch from 914ac1c to 3d2877f Compare November 22, 2018 18:29
@yceruto yceruto force-pushed the request_files_collector branch from 3d2877f to 2e746bc Compare November 22, 2018 18:33
@yceruto
Copy link
Member Author

yceruto commented Nov 22, 2018

@ro0NL comments addressed, thanks!

@nicolas-grekas nicolas-grekas added this to the 4.2 milestone Nov 22, 2018
@fabpot fabpot force-pushed the request_files_collector branch from 2e746bc to 38692a6 Compare November 24, 2018 07:32
@fabpot
Copy link
Member

fabpot commented Nov 24, 2018

Thank you @yceruto.

@fabpot fabpot merged commit 38692a6 into symfony:master Nov 24, 2018
fabpot added a commit that referenced this pull request Nov 24, 2018
…mime type instead of guessing it again (yceruto)

This PR was squashed before being merged into the 4.2-dev branch (closes #29285).

Discussion
----------

[HttpKernel][WebProfilerBundle] Getting the cached client mime type instead of guessing it again

| Q             | A
| ------------- | ---
| Branch?       | master (4.2)
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29284
| License       | MIT
| Doc PR        | -

The 1st commit fixes the bug based on the current implementation. But I'd like to improve this new feature a little bit with the 2nd commit:
 * <del>Renaming "Uploaded files" title by "FILES Parameters" and</del> (sub-section of POST parameters) changing the table using key/value structure (being consistent with other sections).
 * The above allow us also to show nested paramaters. (It is useful to know where this file(s) comes from)
 * And show all info about the `UploadedFile` object. (It might be useful too)

![files-parameters](https://user-images.githubusercontent.com/2028198/48918478-d3a3e100-ee5a-11e8-8ef7-a50ae4ee1550.png)

Commits
-------

38692a6 [HttpKernel][WebProfilerBundle] Getting the cached client mime type instead of guessing it again
@yceruto yceruto deleted the request_files_collector branch November 24, 2018 14:02
This was referenced Nov 26, 2018
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.

6 participants