[Fixes #1791]Remove XML formatter from defaults #1798
Conversation
| if (options.IgnoreBrowserAcceptHeader | ||
| && HasWildCardMediaAndSubMediaType(sortedAcceptHeaderMediaTypes)) | ||
| { | ||
| ignoreAcceptHeader = true; |
There was a problem hiding this comment.
It's typically better to use positive logic, so perhaps rename to RespectBrowserAcceptHeader and respectAcceptHeader respectively
|
The strongly typed headers PR is incoming and it's going to massively conflict with this one. |
| /// <param name="options"></param> | ||
| public static void AddXmlDataContractSerializerFormatter(this MvcOptions options) | ||
| { | ||
| options.OutputFormatters.Add( |
There was a problem hiding this comment.
@kulmugdha you will need to base your changes on top of this
|
Updated /cc: @yishaigalatzer ...I did not address the comment related to a lazy property as it has problems with in-memory tests...will discuss with you in person.. |
|
Rebase and try again.... |
| IOutputFormatter selectedFormatter = null; | ||
|
|
||
| bool respectAcceptHeader = true; |
kichalla
force-pushed
the
remove-xmlformatter
branch
from
bf2c6d8
to
88242f1
Compare
| { | ||
| // Arrange | ||
| var objectResult = new ObjectResult(new Person() { Name = "John" }); | ||
| objectResult.ContentTypes.Add(MediaTypeHeaderValue.Parse("application/xml")); |
There was a problem hiding this comment.
I do not see the produces attribute being used directly here. ( I know it will result in the same thing), I suggest use produces in a functional test to cover this scenario and change the test name to reflect what it actually tests.
There was a problem hiding this comment.
Added some functional tests for this
|
⌚ |
kichalla
force-pushed
the
remove-xmlformatter
branch
from
88242f1
to
24f2778
Compare
kichalla
force-pushed
the
remove-xmlformatter
branch
from
24f2778
to
99a05d1
Compare
|
Updated. @harshgMSFT @yishaigalatzer |
| var server = TestServer.Create(_provider, _app); | ||
| var client = server.CreateClient(); | ||
| client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/xml")); | ||
| client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("*/*", 0.2)); |
There was a problem hiding this comment.
the default accept headers from the browser come without a quality factor, our tests should reflect it.
There was a problem hiding this comment.
actually they do come with quality factor in many cases...example below:
Chrome:
Accept:text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
IE:
Accept: text/html, application/xhtml+xml, */*
Firefox:
Accept:text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Safari:
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Opera:
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
|
fix comments then |
from me.
|
|
|
Commit: 02f4ca9 |
@yishaigalatzer @harshgMSFT