Bug 38334 - Allow other ports to compile ATSUI and CoreText functions in SimpleFontData for Mac.
Summary: Allow other ports to compile ATSUI and CoreText functions in SimpleFontData f...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-29 10:53 PDT by Kevin Ollivier
Modified: 2010-04-30 11:47 PDT (History)
4 users (show)

See Also:

Attachments
Patch (16.67 KB, patch)
2010-04-29 11:06 PDT, Kevin Ollivier 		no flags Details | Formatted Diff | Diff
Patch (88.48 KB, patch)
2010-04-29 16:00 PDT, Kevin Ollivier 		no flags Details | Formatted Diff | Diff
Patch (90.33 KB, patch)
2010-04-29 16:07 PDT, Kevin Ollivier 		no flags Details | Formatted Diff | Diff
Patch (90.59 KB, patch)
2010-04-29 16:11 PDT, Kevin Ollivier 		mitz: review+
Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin Ollivier 2010-04-29 10:53:54 PDT 
Allow other ports to compile ATSUI and CoreText functions in SimpleFontData for Mac.
Comment 1 Kevin Ollivier 2010-04-29 11:06:24 PDT 
Created attachment 54719 [details]
Patch
Comment 2 Kevin Ollivier 2010-04-29 11:25:02 PDT 
This is needed so that wx can compile and use ComplexTextController, BTW.
Comment 3 mitz 2010-04-29 11:28:47 PDT 
Comment on attachment 54719 [details]
Patch

This should be done by svn cp’ing SimpleFontDataMac.mm to the two new files and removing everything but the needed methods, includes and copyright headers. The #if USE()s should go outside the namespace and the second paragraph of #includes.
Comment 4 Kevin Ollivier 2010-04-29 16:00:54 PDT 
Created attachment 54747 [details]
Patch
Comment 5 WebKit Review Bot 2010-04-29 16:03:42 PDT 
Attachment 54747 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/platform/graphics/mac/SimpleFontDataCoreText.cpp:48:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/mac/SimpleFontDataCoreText.cpp:74:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/graphics/mac/SimpleFontDataCoreText.cpp:81:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 3 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 mitz 2010-04-29 16:05:33 PDT 
Comment on attachment 54747 [details]
Patch

Is there a reason why you didn’t address my second feedback item, regarding the placement of the #if directives?
Comment 7 Kevin Ollivier 2010-04-29 16:07:45 PDT 
Created attachment 54749 [details]
Patch
Comment 8 Kevin Ollivier 2010-04-29 16:08:51 PDT 
(In reply to comment #6)
> (From update of attachment 54747 [details])
> Is there a reason why you didn’t address my second feedback item, regarding the
> placement of the #if directives?

Oops, sorry, no, I just got caught up in redoing the patch and forgot about it. I'll fix and re-upload.
Comment 9 Kevin Ollivier 2010-04-29 16:11:02 PDT 
Created attachment 54751 [details]
Patch
Comment 10 Kent Tamura 2010-04-29 23:48:07 PDT 
The patch was committed as r58557, and rolled out by r58560 because of compile errors on Tiger, Leopard, and Chromium/Mac.
Comment 11 Kent Tamura 2010-04-29 23:49:49 PDT 
(In reply to comment #10)
> The patch was committed as r58557, and rolled out by r58560 because of compile
> errors on Tiger, Leopard, and Chromium/Mac.

Correction: Tiger was ok.  Leopard and Chromium/Mac failed.
Comment 12 Jeremy Moskovich 2010-04-30 04:44:39 PDT 
I haven't looked into the compile failures but FYI Chromium compiles with both the ATSUI & Core text functionality enabled and switches the APIs used at runtime.  This, as opposed to other ports that make the decision at compile time.
Comment 13 WebKit Review Bot 2010-04-30 09:51:13 PDT 
http://trac.webkit.org/changeset/58581 might have broken Chromium Mac Release
Comment 14 Kevin Ollivier 2010-04-30 11:47:11 PDT 
Landed in r58581, with Chromium build fix in r58582.