[webkit-reviews] review requested: [Bug 27734] WINCE PORT: font related changes : [Attachment 34716] build fixes and minor bug fixes, with more comments

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 12 22:49:08 PDT 2009


Joe Mason <joe.mason at torchmobile.com> has asked  for review:
Bug 27734: WINCE PORT: font related changes
https://bugs.webkit.org/show_bug.cgi?id=27734

Attachment 34716: build fixes and minor bug fixes, with more comments
https://bugs.webkit.org/attachment.cgi?id=34716&action=review

------- Additional Comments from Joe Mason <joe.mason at torchmobile.com>
> Are you sure that min<int> will compile on other platforms?  I feel like
we've
> dealt with the min/max issue on windows before in other ways.  Please grep
the
> source to see how it's dealt with elsewhere.

The more common methods of dealing with the windows min/max macros won't work
here because the two parameters to min have different types (int and UChar32),
so Windows gives an error about ambiguous template parameters.	min<int> will
definitely compile on other platforms (it's just giving the template parameter
explicitly instead of letting the compiler work it out), but I changed the fix
anyway because I felt it was clearer - now it explicitly casts the UChar32 to
an int.

> This needs a why comment:
> 29 #if !PLATFORM(WINCE)
> 227230     mutable SCRIPT_CACHE m_scriptCache;
> 228231     mutable SCRIPT_FONTPROPERTIES* m_scriptFontProperties;
> 229232 #endif
> your note about it saving space in the ChangeLog is a fine comment, just put
it
> in teh soruce code too.

Done.


More information about the webkit-reviews mailing list