[webkit-reviews] review denied: [Bug 61528] Implement maxWidth check for canvas fill/strokeText : [Attachment 101247] Fixes the canvas/philip/tests/2d.text.draw.fill.maxWidth.fontface.html test

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied Philip Rogers
<pdr at google.com>'s request for review:
Bug 61528: Implement maxWidth check for canvas fill/strokeText
https://bugs.webkit.org/show_bug.cgi?id=61528

Attachment 101247: Fixes the
canvas/philip/tests/2d.text.draw.fill.maxWidth.fontface.html test
https://bugs.webkit.org/attachment.cgi?id=101247&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=101247&action=review


Please don't check in _another_ copy of Ahem.ttf.  We already have 5 copies in
the tree, refer to one of those or (even better) move one of the existing
copies to a more common directory and reference it.  Maybe
LayoutTests/resources/ ?

> Source/WebCore/ChangeLog:1
> +2011-07-09  Philip Rogers  <pdr>

Please put your full email address in the changelog.

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1937
> +    if (fontWidth <= 0)

This is technically incorrect, although I doubt we have any tests that cover
this case.  If the actual width of the text is == 0, we still have to apply the
globalCompositeOperation even though this draw call does not touch any pixels
because of the way compositing operations are defined in canvas 2d.  For
example, if the globalCompositeOperation is "copy" doing a fillText() with text
that occupies 0 width should result in a fully transparent black canvas. 
http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.
html#drawing-model for reference.

> LayoutTests/ChangeLog:1
> +2011-07-09  Philip Rogers  <pdr>

Need full email