Last Comment Bug 765704 - Don't throw when setting XHR.responseType to an invalid value
: Don't throw when setting XHR.responseType to an invalid value
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: :Ms2ger
:
Mentors:
Depends on:
Blocks: 740069
  Show dependency treegraph
 
Reported: 2012-06-18 05:49 PDT by :Ms2ger
Modified: 2013-04-04 13:53 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
+
verified
+
fixed


Attachments
Patch v1 (9.82 KB, patch)
2012-06-18 08:15 PDT, :Ms2ger
bzbarsky: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Review

Description :Ms2ger 2012-06-18 05:49:56 PDT
The algorithm at <http://dev.w3.org/2006/webapi/WebIDL/#dfn-attribute-setter> explicitly special-cases enums, so that setting an enum attribute to an unrecognized value silently fails, rather than throwing. We broke that.
Comment 1 Kyle Huey [:khuey] (khuey@mozilla.com) (Away until 6/13) 2012-06-18 05:53:22 PDT
We have to fix this before release.
Comment 2 :Ms2ger 2012-06-18 08:15:50 PDT
Created attachment 634043 [details] [diff] [review]
Patch v1

This isn't terribly elegant, but it seems to work, at least.
Comment 3 Boris Zbarsky [:bz] 2012-06-18 08:29:07 PDT
Comment on attachment 634043 [details] [diff] [review]
Patch v1

I don't think you need the invalidEnumValueFatal function.  Just pass "not setter" in CGPerSignatureCall.__init__ ?

r=me.  Please add mochtests for XHR here or something to actually exercise this code?  Also tests for the fact that the method argument throws...
Comment 4 :Ms2ger 2012-06-18 13:05:05 PDT
Comment on attachment 634043 [details] [diff] [review]
Patch v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 740069
User impact if declined: scripts can break if they set responseType to something unrecognized
Testing completed (on m-c, etc.): passes try <https://tbpl.mozilla.org/?tree=Try&rev=ac41076a1718>
Risk to taking this patch (and alternatives if risky): small
String or UUID changes made by this patch: none
Comment 5 Lukas Blakk [:lsblakk] use ?needinfo 2012-06-18 15:44:25 PDT
Looks like we'll get this into all branches with plenty of time but setting tracking anyway in case there are any followup issues.
Comment 6 Lukas Blakk [:lsblakk] use ?needinfo 2012-06-18 15:50:33 PDT
Comment on attachment 634043 [details] [diff] [review]
Patch v1

low risk fix, test included, and we're still nice and early in the beta cycle - approving.

Note You need to log in before you can comment on or make changes to this bug.