NEW 32443
Whether to send XHTMLMP ACCEPT header for network requests should be configurable 
https://bugs.webkit.org/show_bug.cgi?id=32443
Summary Whether to send XHTMLMP ACCEPT header for network requests should be configur...
Adam Treat
Reported 2009-12-11 12:50:04 PST
Currently, if ENABLE(XHTMLMP) == true, then the ACCEPT header for network request is hardcoded to include mobile content types. This should be runtime configurable as just because WebKit has support for XHTMLMP in a particular build does not mean that it should always be used.
Attachments
Adds a new setting (4.53 KB, patch)
2009-12-11 13:07 PST, Adam Treat 		levin: review-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Adam Treat
Comment 1 2009-12-11 13:07:09 PST
Created attachment 44705 [details] Adds a new setting Adds a new runtime setting for enable/disable of xhtmlmp content.
WebKit Review Bot
Comment 2 2009-12-11 13:12:28 PST
style-queue ran check-webkit-style on attachment 44705 [details] without any errors.
Eric Seidel (no email)
Comment 3 2009-12-11 15:37:05 PST
Comment on attachment 44705 [details] Adds a new setting This should have a test, no? We could test this with things like layoutTestController.overridePreference()
Adam Treat
Comment 4 2009-12-11 15:51:57 PST
You'd need a server configured to respond to the ACCEPT header. We don't have that.
Eric Seidel (no email)
Comment 5 2009-12-11 15:58:41 PST
Hum... I take it our Apache2 setup doesn't do that?
Adam Treat
Comment 6 2009-12-11 16:04:59 PST
(In reply to comment #5) > Hum... I take it our Apache2 setup doesn't do that? I'll take a look and see.
Eric Seidel (no email)
Comment 7 2009-12-11 16:09:26 PST
Thank you.
Alexey Proskuryakov
Comment 8 2009-12-11 16:43:17 PST
Comment on attachment 44705 [details] Adds a new setting There seems to be some confusion (not sure if it's in the bug, in the patch, or just in my head) about what this change does. If it's only about changing Accept header, then the setting should not be called XHTMLMPEnabled. And if it disables XHTMLMP support as a whole, then bug and description should say so. Style nit - setXHTMLMPEnabled is fine, but XHTMLMPEnabled should be xhtmlmpEnabled, or maybe xhtmlMPEnabled.
David Levin
Comment 9 2009-12-17 15:53:28 PST
Comment on attachment 44705 [details] Adds a new setting It sounds like there a number of issues in the bug that need to be addressed (or responded to) before this can be reviewed, so I'm marking this as r- for now to move this out of the review queue. Also, please add a link to the bug in the changelog.
Note You need to log in before you can comment on or make changes to this bug.