[Webkit-unassigned] [Bug 142694] Update some canvas tests and fix a related bug with unavailable images

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Mar 15 16:06:34 PDT 2015


Chris Dumez <cdumez at apple.com> changed:

           What    |Removed                     |Added
 Attachment #248686|review?                     |review-
              Flags|                            |

--- Comment #13 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 248686
  --> https://bugs.webkit.org/attachment.cgi?id=248686

View in context: https://bugs.webkit.org/attachment.cgi?id=248686&action=review

> Source/WebCore/ChangeLog:10
> +        these tests and fixes a canvas bug the new tests exposed.

Please explain in detail which bug you fixed and how you changed Web-Exposed behavior. Pointing to a spec and comparing the new behavior to other browsers would also be helpful.

> Source/WebCore/ChangeLog:11
> +

Please indicate in the ChangeLog where you got the new tests from. You can also include the W3C bug URL that explains which tests were replaced by which.

> Source/WebCore/html/HTMLImageElement.h:81
> +    bool unavailable() const;

Ideally, as per coding style, this would be called isUnavaiable(). I would also rename complete() to isComplete() for consistency.

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:1754
> +    if (image->unavailable())

I don't see any mention of the unavailable state at https://html.spec.whatwg.org/#check-the-usability-of-the-image-argument
Could you clarify why this is the right thing to do here?

> Source/WebCore/loader/ImageLoader.cpp:155
> +        m_imageUnavailable = false;

The spec says we are supposed to move out of "unavailable" state as soon as we have enough data to determine the image's dimensions (https://html.spec.whatwg.org/#img-none). Aren't we moving out of unavailable state too late here?

> Source/WebCore/loader/ImageLoader.cpp:284
> +    m_imageUnavailable = false;

We should have moved out of "unavailable" state as soon as we got enough data to know the image dimensions.

> LayoutTests/ChangeLog:25
> +        This test currently fails and is fixed in https://bugs.webkit.org/show_bug.cgi?id=142677

"and will be fixed in..."

> LayoutTests/TestExpectations:501
> +webkit.org/b/142677 canvas/philip/tests/2d.pattern.image.incomplete.removedsrc.html [ Pass Failure ]

Why [ Pass, Failure] ? This means flaky. Either commit the incorrect expectations (I think this would be OK since you are fixing the test in a follow-up patch) or commit the correct expectations and mark the test as [ Failure ] (ideal).

> LayoutTests/canvas/philip/tests/2d.pattern.image.incomplete.removedsrc-expected.txt:8
> +file:///Users/yweiss/OS/WebKit/LayoutTests/canvas/philip/tests/2d.pattern.image.incomplete.removedsrc.html:24:30

These absolute paths are going to be an issue. Either commit the "correct" expected result or tweak the test so that it does not print absolute paths.

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150315/1534c857/attachment-0002.html>

More information about the webkit-unassigned mailing list