Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1734853002:
Sync createPattern with the spec (Closed)
|
Created:
4 years, 10 months ago by rwlbuis Modified:
4 years, 10 months ago Reviewers:
philipj_slow CC:
chromium-reviews, blink-reviews, ajuma+watch-canvas_chromium.org, Rik, Justin Novosad, dshwang Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionSync createPattern with the spec
According to the spec the repetition argument is not nullable, but
the return type is:
https://html.spec.whatwg.org/multipage/scripting.html#canvasfillstrokestyles
This change is covered by fast/canvas/canvas-pattern-behaviour.html.
No change in behavior for the repitition parameter since nullable and
TreatNullAs=NullString both convert null to a null string.
BUG=460722
Committed: https://crrev.com/90717942048c6aa1a9f56983b9a16eb7b7a50961
Cr-Commit-Position: refs/heads/master@{#377523}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Make return type nullable #Messages
Total messages: 15 (8 generated)
Description was changed from ========== pattern WIP BUG= ========== to ========== Sync createPattern with the spec According to the spec the repition argument is not nullable: https://html.spec.whatwg.org/multipage/scripting.html#canvasfillstrokestyles BUG=460722 ==========
Description was changed from ========== Sync createPattern with the spec According to the spec the repition argument is not nullable: https://html.spec.whatwg.org/multipage/scripting.html#canvasfillstrokestyles BUG=460722 ========== to ========== Sync createPattern with the spec According to the spec the repetition argument is not nullable: https://html.spec.whatwg.org/multipage/scripting.html#canvasfillstrokestyles BUG=460722 ==========
rob.buis@samsung.com changed reviewers: + philipj@opera.com
PTAL.
LGTM, but can you explain in the description why this change is not observable at all and point to the tests that already cover this? https://codereview.chromium.org/1734853002/diff/1/third_party/WebKit/Source/m... File third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl (right): https://codereview.chromium.org/1734853002/diff/1/third_party/WebKit/Source/m... third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl:72: [RaisesException] CanvasPattern createPattern(CanvasImageSource image, [TreatNullAs=NullString] DOMString repetitionType); While you're here, can you make the return type nullable? That shouldn't change the generated code at all.
Description was changed from ========== Sync createPattern with the spec According to the spec the repetition argument is not nullable: https://html.spec.whatwg.org/multipage/scripting.html#canvasfillstrokestyles BUG=460722 ========== to ========== Sync createPattern with the spec According to the spec the repetition argument is not nullable, but the return value is: https://html.spec.whatwg.org/multipage/scripting.html#canvasfillstrokestyles BUG=460722 ==========
Description was changed from ========== Sync createPattern with the spec According to the spec the repetition argument is not nullable, but the return value is: https://html.spec.whatwg.org/multipage/scripting.html#canvasfillstrokestyles BUG=460722 ========== to ========== Sync createPattern with the spec According to the spec the repetition argument is not nullable, but the return type is: https://html.spec.whatwg.org/multipage/scripting.html#canvasfillstrokestyles This change is covered by fast/canvas/canvas-pattern-behaviour.html. No change in behavior for the repitition parameter since nullable and TreatNullAs=NullString both convert null to a null string. BUG=460722 ==========
On 2016/02/25 03:01:46, philipj_UTC7 wrote: > LGTM, but can you explain in the description why this change is not observable > at all and point to the tests that already cover this? Done! > https://codereview.chromium.org/1734853002/diff/1/third_party/WebKit/Source/m... > File third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl > (right): > > https://codereview.chromium.org/1734853002/diff/1/third_party/WebKit/Source/m... > third_party/WebKit/Source/modules/canvas2d/CanvasRenderingContext2D.idl:72: > [RaisesException] CanvasPattern createPattern(CanvasImageSource image, > [TreatNullAs=NullString] DOMString repetitionType); > While you're here, can you make the return type nullable? That shouldn't change > the generated code at all. Good point, I made that change and updated the description.
Thanks! Even more LGTM.
The CQ bit was checked by rob.buis@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1734853002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1734853002/20001
Message was sent while issue was closed.
Description was changed from ========== Sync createPattern with the spec According to the spec the repetition argument is not nullable, but the return type is: https://html.spec.whatwg.org/multipage/scripting.html#canvasfillstrokestyles This change is covered by fast/canvas/canvas-pattern-behaviour.html. No change in behavior for the repitition parameter since nullable and TreatNullAs=NullString both convert null to a null string. BUG=460722 ========== to ========== Sync createPattern with the spec According to the spec the repetition argument is not nullable, but the return type is: https://html.spec.whatwg.org/multipage/scripting.html#canvasfillstrokestyles This change is covered by fast/canvas/canvas-pattern-behaviour.html. No change in behavior for the repitition parameter since nullable and TreatNullAs=NullString both convert null to a null string. BUG=460722 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Sync createPattern with the spec According to the spec the repetition argument is not nullable, but the return type is: https://html.spec.whatwg.org/multipage/scripting.html#canvasfillstrokestyles This change is covered by fast/canvas/canvas-pattern-behaviour.html. No change in behavior for the repitition parameter since nullable and TreatNullAs=NullString both convert null to a null string. BUG=460722 ========== to ========== Sync createPattern with the spec According to the spec the repetition argument is not nullable, but the return type is: https://html.spec.whatwg.org/multipage/scripting.html#canvasfillstrokestyles This change is covered by fast/canvas/canvas-pattern-behaviour.html. No change in behavior for the repitition parameter since nullable and TreatNullAs=NullString both convert null to a null string. BUG=460722 Committed: https://crrev.com/90717942048c6aa1a9f56983b9a16eb7b7a50961 Cr-Commit-Position: refs/heads/master@{#377523} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/90717942048c6aa1a9f56983b9a16eb7b7a50961 Cr-Commit-Position: refs/heads/master@{#377523} |
