Bug 32987

Summary: Add build option ENABLE_XHTMLMP to Mac, GTK, and Chromium build systems
Product: WebKit Reporter: Daniel Bates <dbates>
Component: Tools / TestsAssignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben, dglazkov, eric, jmalonzo, mrowe, ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Macintosh   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch eric: review+, eric: commit-queue-

Description Daniel Bates 2009-12-28 09:57:11 PST
We should add the build option to enable XHTML-MP support to the Mac, GTK, and Chromium build systems (disabled by default).

Currently, passing the command-line argument --enable-xhtmlmp to build-webkit is only honored by the Qt and Windows builds.
Comment 1 Daniel Bates 2010-01-04 12:16:41 PST
Created attachment 45815 [details]
Patch
Comment 2 WebKit Review Bot 2010-01-04 12:17:28 PST
style-queue ran check-webkit-style on attachment 45815 [details] without any errors.
Comment 3 WebKit Review Bot 2010-01-04 12:24:03 PST
Attachment 45815 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/161668
Comment 4 Daniel Bates 2010-01-04 12:36:07 PST
CC'ing people knowledgeable about the various build systems, including Csaba Osztrogonac, Jan Alonzo, Eric Seidel, Adam Roben, and Mark Rowe
Comment 5 Daniel Bates 2010-01-04 12:38:53 PST
Created attachment 45817 [details]
Patch

Added missing comma after 'ENABLE_XHTMLMP=0' in WebKit/chromium/features.gypi.
Comment 6 WebKit Review Bot 2010-01-04 12:43:14 PST
style-queue ran check-webkit-style on attachment 45817 [details] without any errors.
Comment 7 Csaba Osztrogon√°c 2010-01-04 13:44:19 PST
WebKitTools/Scripts/build-webkit script pass to Qt build system if XHTML-MP is enabled or disabled. Now it is disabled by default, you can enable explicitly with: WebKitTools/Scripts/build-webkit --xhtmlmp

If you would like to use different default value for Qt port, you should put one of this line into WebCore.pro

!contains(DEFINES, ENABLE_XHTMLMP=.): DEFINES += ENABLE_XHTMLMP=0
or
!contains(DEFINES, ENABLE_XHTMLMP=.): DEFINES += ENABLE_XHTMLMP=1

Irrespectively of this modification, you can use --xhtmlmp and --no-xhtmlmp to toggle XHTML-MP support.
Comment 8 Eric Seidel (no email) 2010-01-05 13:25:00 PST
Comment on attachment 45817 [details]
Patch

Assuming HTMLNoScriptElement is appropriately guarded, this looks fine.

configure.ac looks insanely verbose and redundant.

Your ChangeLogs need lovin before you can land this:
+        Need a short description and bug URL (OOPS!)
+
+        No new tests. (OOPS!)
+
Comment 9 Daniel Bates 2010-01-07 21:14:10 PST
(In reply to comment #8)
> (From update of attachment 45817 [details])
> Assuming HTMLNoScriptElement is appropriately guarded, this looks fine.
> 
> configure.ac looks insanely verbose and redundant.

We should probably look to clean this up in a separate bug.

> 
> Your ChangeLogs need lovin before you can land this:
> +        Need a short description and bug URL (OOPS!)
> +
> +        No new tests. (OOPS!)
> +

Will change.
Comment 10 Daniel Bates 2010-01-07 22:16:55 PST
Committed r52972: <http://trac.webkit.org/changeset/52972>