Bug 29143 - Added MathML toggle to build script
: Added MathML toggle to build script
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Tools / Tests
: 528+ (Nightly build)
: Macintosh Intel Mac OS X 10.5
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks: 29158
  Show dependency treegraph
 
Reported: 2009-09-10 13:03 PDT by Alex Milowski
Modified: 2009-09-15 13:34 PDT (History)
2 users (show)

See Also:


Attachments
Patch to add toggle flag to build script (1.86 KB, patch)
2009-09-10 13:04 PDT, Alex Milowski
eric: review-
Details | Formatted Diff | Diff
Updated patch with tab removed (1.86 KB, patch)
2009-09-10 13:31 PDT, Alex Milowski
mrowe: review-
Details | Formatted Diff | Diff
Updated patch with sorted order for additions (1.97 KB, patch)
2009-09-11 07:05 PDT, Alex Milowski
vestbo: review-
Details | Formatted Diff | Diff
Updated patch with FeatureDefines.xcconfig changes included (9.22 KB, patch)
2009-09-15 13:17 PDT, Alex Milowski
vestbo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Milowski 2009-09-10 13:03:36 PDT
Added a toggle to the script to set the ENABLE_MATHML define.
Comment 1 Alex Milowski 2009-09-10 13:04:30 PDT
Created attachment 39369 [details]
Patch to add toggle flag to build script
Comment 2 Eric Seidel 2009-09-10 13:25:42 PDT
Comment on attachment 39369 [details]
Patch to add toggle flag to build script

Doesn't ENABLE_ need to be added to http://trac.webkit.org/browser/trunk/WebCore/Configurations/FeatureDefines.xcconfig too?  I'm not sure what the rules are.  Mark Rowe (bdash) would know.
Comment 3 Eric Seidel 2009-09-10 13:26:10 PDT
Comment on attachment 39369 [details]
Patch to add toggle flag to build script

Tabs in your ChangeLog will make this unlandable, r-
Comment 4 Alex Milowski 2009-09-10 13:31:10 PDT
Created attachment 39371 [details]
Updated patch with tab removed
Comment 5 Alex Milowski 2009-09-10 13:32:43 PDT
I have another patch that will use the ENABLE_MATHML.  I don't see what an extra define that isn't used is going to hurt.  When I submit the CSS patch and rendering patches, it will be added to the WebCore/Configurations/FeatureDefines.xconfig file.
Comment 6 Mark Rowe (bdash) 2009-09-10 22:18:40 PDT
Comment on attachment 39371 [details]
Updated patch with tab removed

The variable and option declarations are in alphabetical order.  The new variable and option should maintain that.  I think it would be best for MathML to be disabled by default until the implementation is more complete and tested.  Other than those minor issues, this change is fine.


Marking as r- as a revised patch will need to be posted before we can commit it.
Comment 7 Alex Milowski 2009-09-11 07:05:15 PDT
Created attachment 39429 [details]
Updated patch with sorted order for additions
Comment 8 Tor Arne Vestbø 2009-09-15 12:30:00 PDT
Comment on attachment 39429 [details]
Updated patch with sorted order for additions

LGTM
Comment 9 Tor Arne Vestbø 2009-09-15 12:37:31 PDT
Comment on attachment 39429 [details]
Updated patch with sorted order for additions

r- per bdash's comment about also updating FeatureDefines.xcconfig
Comment 10 Alex Milowski 2009-09-15 13:17:52 PDT
Created attachment 39612 [details]
Updated patch with FeatureDefines.xcconfig changes included
Comment 11 Tor Arne Vestbø 2009-09-15 13:24:06 PDT
Comment on attachment 39612 [details]
Updated patch with FeatureDefines.xcconfig changes included

r=me
Comment 12 Tor Arne Vestbø 2009-09-15 13:34:30 PDT
Landed in r48400