Closed Bug 784809 Opened 12 years ago Closed 12 years ago

createPattern should allow null for the repeat argument

Categories

(Core :: Graphics: Canvas2D, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17
Tracking Status
firefox16 + fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

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.
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.
Assignee: nobody → peterv
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.
Assignee: peterv → 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.
I filed bug 784869 to do the "throw on undefined" thing.
Whiteboard: [need review]
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>.
Attachment #654444 - Flags: review?(peterv) → review+
> 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.
Sigh... Can we at least not use "the spec" to refer to the W3C fork without qualification?
<shrug>. I've lost tracks of all the various "specs" involved.
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla17
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.
Attachment #654444 - Flags: approval-mozilla-aurora?
Sorry, I backed this out because test_canvas.html was failing on Linux. https://hg.mozilla.org/integration/mozilla-inbound/rev/2266e0721327
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.
Attachment #654796 - Flags: approval-mozilla-aurora?
Attachment #654444 - Attachment is obsolete: true
Attachment #654444 - Flags: approval-mozilla-aurora?
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
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.
Attachment #654796 - Flags: approval-mozilla-aurora? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: