[webkit-reviews] review denied: [Bug 108881] Canvas fillText and measureText handle ideographic spaces differently : [Attachment 208414] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Aug 9 04:27:18 PDT 2013
Christophe Dumez <dchris at gmail.com> has denied Rashmi Shyamasundar
<rashmi.s2 at samsung.com>'s request for review:
Bug 108881: Canvas fillText and measureText handle ideographic spaces
differently
https://bugs.webkit.org/show_bug.cgi?id=108881
Attachment 208414: Patch
https://bugs.webkit.org/attachment.cgi?id=208414&action=review
------- Additional Comments from Christophe Dumez <dchris at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=208414&action=review
> Source/WebCore/ChangeLog:12
> + This patch modifies the canvas functions drawTextInternal and
measureText to conform to the above spec.
Does it match Firefox as well?
> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:93
>
Would be nice to add a link to the specification in comment:
http://www.whatwg.org/specs/web-apps/current-work/multipage/common-microsyntaxe
s.html#space-character
> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:94
> +static inline bool shouldSpaceBeReplaced(UChar c)
This function has nothing to do with replacing. Maybe we can call it
isHTMLSpace() ?
> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:96
> + return (c == 0x0009 || c == 0x000A || c == 0x000B || c == 0x000C || c ==
0x000D) ? true : false;
return (c == 0x0009 || c == 0x000A || c == 0x000B || c == 0x000C || c ==
0x000D);
> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:2111
> + const size_t replacementLength = replacement.length();
length() returns an 'unsigned' type. I think it is best to use the same type.
> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:2112
> + size_t index = 0;
No need to initialize to 0.
> LayoutTests/ChangeLog:11
> + The below listed layout tests veriy the conformance to above spec.
verify
> LayoutTests/fast/canvas/canvas-fillText-ideographicSpace-expected.html:1
> +<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<!DOCTYPE html>
More information about the webkit-reviews
mailing list