784809 - createPattern should allow null for the repeat argument

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Apparently it used to, and other UAs do that too.  And the spec changed recently to allow it.

This means backing out part of bug 762657.

We should probably try to get this into aurora.

Comment 1

[Alex Keybl [:akeybl]]

If peterv agrees, I have no issue with backing out bug 762657 prior to the merge.

If not, I'd need to know more context about the user impact. Sounds like we're just not ready to meet the spec though.

Comment 2

[Boris Zbarsky [:bzbarsky]]

We don't want to back out bug 762657.  We just need to back out the xpidl change and part of the test changes.

The user impact is that createPattern calls that used to work and should work per the updated spec throw on m-c and aurora right now.

I'm going to have a patch for this very soon.

Comment 3

[Boris Zbarsky [:bzbarsky]]

I'm going to make this nullable for now in the WebIDL, but the right solution is actually [TreatNullAs=EmptyString].  That would need bug 742195, which is not happening on Aurora.

Comment 4

[Boris Zbarsky [:bzbarsky]]

I filed bug 784869 to do the "throw on undefined" thing.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Allow null for the repeat argument of canvas createPattern.

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

The spec hasn't changed to allow it, and Opera throws as well. See <https://www.w3.org/Bugs/Public/show_bug.cgi?id=17891>.

Comment 7

[Boris Zbarsky [:bzbarsky]]

> The spec hasn't changed to allow it

Rik's editor's draft has...

I believe we should take this to mitigate risk for shipping the webidl canvas stuff, personally.  And I don't see the benefit of switching behavior in a way that might break web pages, honestly.

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Sigh... Can we at least not use "the spec" to refer to the W3C fork without qualification?

Comment 9

[Boris Zbarsky [:bzbarsky]]

<shrug>.  I've lost tracks of all the various "specs" involved.

Comment 10

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b88ccf7bd3e3

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 654444 [details] [diff] [review]
Allow null for the repeat argument of canvas createPattern.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 762657
User impact if declined: Some sites might throw exceptions that they don't expect
Testing completed (on m-c, etc.): Passes our tests, as far as I can tell.
Risk to taking this patch (and alternatives if risky): Risk is low.  Just allows
   passing null (and undefined) for the second argument to createPattern,
   basically restoring the Firefox 15 behavior.
String or UUID changes made by this patch: None.

Comment 12

[Matt Brubeck (:mbrubeck)]

Sorry, I backed this out because test_canvas.html was failing on Linux.
https://hg.mozilla.org/integration/mozilla-inbound/rev/2266e0721327

Comment 13

[Boris Zbarsky [:bzbarsky]]

Gah.  Because that's a non-azure canvas, so the test result is different for the moment.  I'm just going to comment that test for undefined out for now.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: With the test fixed on non-Azure canvas

Comment 15

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1bed30d952cc

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1bed30d952cc

Comment 17

[Alex Keybl [:akeybl]]

Comment on attachment 654796 [details] [diff] [review]
With the test fixed on non-Azure canvas

[Triage Comment]
Low risk regression fix, approved for Beta 16.

Comment 18

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/releases/mozilla-beta/rev/57ce663f5d6f