Bug 16994 – SmartReplace.cpp isn't implemented for platforms without CoreFoundation

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Tony Chang]

The method WebCore::isCharacterSmartReplaceExempt is needed for smart paste to work.  It's currently unimplemented unless CoreFoundation is in the build.  I have a patch coming up that implements this method using ICU directly.

[Tony Chang]

Created attachment 18638 [details]
implement WebCore::isCharacterSmartReplaceExempt

This is mostly translation of the method found in SmartReplaceCF.cpp.  No new tests since this behavior is tested by existing layout tests.

[Darin Adler]

Comment on attachment 18638 [details]
implement WebCore::isCharacterSmartReplaceExempt

Code looks fine, but it's not platform-independent since it uses ICU; not all ports of WebCore have ICU, and it's not acceptable to just have ICU code without any ifdefs. It should go into a file named SmartReplaceICU.cpp and be properly ifdef'd. Otherwise, fine.

+        // Nice to have, added in ICU 3.8.
+        //uset_freeze(smartSet);

This can be ifdef'd based on the ICU headers. It would be better to either include it for real or not. We don't normally include commented out code.

The if is "#if USE (ICU_UNICODE)" I believe. You should look for an example.

[Tony Chang]

Created attachment 18752 [details]
take 2

Same as before with #ifdef USE(ICU_UNICODE) guards.

[Eric Seidel (no email)]

Comment on attachment 18752 [details]
take 2

This still should be in its own SmartReplaceICU.cpp instead of an #ifdef in SmartReplace.cpp.  We're trying to kill all the #ifdefs over time.

[Darin Adler]

Comment on attachment 18752 [details]
take 2

+#ifdef USE(ICU_UNICODE)

That's incorrect. It needs to be:

#if USE(ICU_UNICODE)

And the ICU version should really be in a different file, as the CF version is.

review- because of the ifdef/if issue.

[Darin Adler]

Comment on attachment 18752 [details]
take 2

+#ifdef USE(ICU_UNICODE)

That's incorrect. It needs to be:

#if USE(ICU_UNICODE)

And the ICU version should really be in a different file, as the CF version is.

review- because of the ifdef/if issue.

[Tony Chang]

Created attachment 18861 [details]
take 3

Fix the preprocessor commands.

[Tony Chang]

Created attachment 18862 [details]
in a separate file

Here's a version with the code in a separate file.  I'm not sure which build config files needed to be updated to pick up the new file (as in, I don't know which builds use ICU and which don't).

[Darin Adler]

Comment on attachment 18861 [details]
take 3

OK. r=me

[Mark Rowe (bdash)]

Landed in r29990 with the necessary tweaks to be used in the Gtk and wxWidget ports.  Thanks for the patch!