Bug 32443

Summary:

Whether to send XHTMLMP ACCEPT header for network requests should be configurable

Product:

WebKit

Reporter:

Adam Treat <manyoso>

Component:

Page Loading

Assignee:

Nobody <webkit-unassigned>

Status:

NEW

Severity:

Normal

CC:

ap, eric, laszlo.gombos, manyoso, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Attachments:

Description	Flags
Adds a new setting	levin: review-

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-	Details Formatted Diff Diff
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.