Implement currentColor for Canvas colors

[dzbarsky]

I made addColorStop throw for currentColor, which matches Firefox. It's easy enough to change the behavior to fetch the color from the canvas, which is what Chrome and Safari do. I'll file a spec bug to decide what to do here.

[Ms2ger]

It's easy enough to change the behavior to fetch the color from the canvas

It actually isn't: these objects are not bound to one particular canvas.

[dzbarsky]

Simon found the spec text in #7106 and I updated the PR to match. It doesn't check whether the canvas is actually being rendered, I'll do that in a followup since I'll need to add a new message and some layout code.

[dzbarsky]

The second commit checks if the canvas is rendered before grabbing the used color for currentColor.
With it, we pass the following testcase:

     var canvas = document.getElementById('canvas');
    var ctx = canvas.getContext('2d');
    canvas.style.color = 'red';
    canvas.style.display = 'none';
    canvas.offsetTop;
    ctx.fillStyle = 'currentColor';
 canvas.style.display = 'inline';
  ctx.fillRect(0, 0, 100, 50);

However, Firefox, Chrome, and Safari all display a red rect instead of a black one, so I think this requirement isn't actually necessary.

[Ms2ger]

I don't see that test case in the PR? ;)

[dzbarsky]

I didn't want to spend effort making it an actual reftest in case we decided not to land the second patch. If you think we should follow the spec here instead of changing it to match browsers, I can add the test.

[Ms2ger]

I'd probably stick with matching current browsers and filing issues. I still want the test, though, with the colours swapped.

[dzbarsky]

Ok, I changed the test a little and added it.

[bors-servo]

☔ The latest upstream changes (presumably #7104) made this pull request unmergeable. Please resolve the merge conflicts.

[bors-servo]

☔ The latest upstream changes (presumably #7416) made this pull request unmergeable. Please resolve the merge conflicts.

[bors-servo]

☔ The latest upstream changes (presumably #7431) made this pull request unmergeable. Please resolve the merge conflicts.

[metajack]

r? @Ms2ger

[SimonSapin]

Reviewed 7 of 7 files at r1.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

^{components/script/dom/canvasgradient.rs, line 53 [r1] (raw file):}
Is this function still used at all? If not, please remove it.

---

^{components/script/dom/canvasrenderingcontext2d.rs, line 426 [r1] (raw file):}
This seems wasteful: it serializes an RGBA struct to a string just to parse it again. Could this code access the ComputedValues struct directly, rather than use GetComputedStyle?

---

^{tests/wpt/web-platform-tests/2dcontext/fill-and-stroke-styles/2d.fillStyle.parse.current.notrendered.html, line 2 [r1] (raw file):}
I don’t see this file upstream. @Ms2ger, would it be removed at the next sync? Should it go to tests/wpt/mozilla instead?

---

Comments from the review on Reviewable.io

[frewsxcv]

@dzbarsky Any updates on this?

[dzbarsky]

Didn't realize this got reviewed, I'll fix it up.

[dzbarsky]

Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

components/script/dom/canvasgradient.rs, line 53 [r1] (raw file):
which function? parse_color is still used in canvasrenderingcontext2d.

---

components/script/dom/canvasrenderingcontext2d.rs, line 426 [r1] (raw file):
It seems like we need a TSLayounNode to access ComputedStyles and I'm not even sure if that's ok to create from script. The complexity doesn't seem worth it for this edge case.

---

tests/wpt/web-platform-tests/2dcontext/fill-and-stroke-styles/2d.fillStyle.parse.current.notrendered.html, line 2 [r1] (raw file):
I believe it gets added for WPT (as opposed to CSS tests)

---

Comments from the review on Reviewable.io

[jdm]

The computed styles question sounds very similar to #7267.

[SimonSapin]

@bors-servo r+

---

Reviewed 3 of 3 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

Comments from the review on Reviewable.io

[bors-servo]

📌 Commit 8408891 has been approved by SimonSapin

[bors-servo]

⌛ Testing commit 8408891 with merge 0bf19b5...

[bors-servo]

💔 Test failed - mac-rel-wpt

[frewsxcv]

./components/script/dom/canvasrenderingcontext2d.rs:444: missing space after ,