Ticket #2531617 (closed enhancement)
Reporter  Patrick Cavit |
Opened: 12/16/11 Last modified: 08/1/12 Status: closed Type: enhancement Resolution: fixed |
Owner  Satyen Desai |
Target Release: 3.6.0 Priority: P3 (normal) |
|---|---|---|---|
| Summary: | Attribute.setAttrs no longer supports passing options | ||
| Description: | In 3.4.1 Attribute.setAttrs supported an undocumented second argument to pass along custom options to be mixed into the This functionality still exists in in master but it's been broken out into AttributeEvents. The problem is that a) it So it seems like AttributeEvents.setAttrs either shouldn't support options, or AttributeCore._setAttrs should support it. I'd prefer it get fixed & have documentation added as I'd like to make use of |
||
| Type: | enhancement | Observed in Version: | development master |
| Component: | Attribute | Severity: | S3 (normal) |
| Assigned To: | Satyen Desai | Target Release: | 3.6.0 |
| Location: | Priority: | P3 (normal) | |
| Tags: | Relates To: | ||
| Browsers: | All | ||
| URL: | |||
| Test Information: | |||
Change History
|
Posted: 12/16/11
|
|
Posted: 12/16/11
Oof, I must have mis-read the 3.4.1 Attribute code last night. That's embarassing. So not a regression, no. Just a weird undocumented param that never did anything & has since been cleaned up. I'd love for it to be added to setAttrs & plumbed through though! |
|
Posted: 12/16/11
Sounds good. Yeah, can't think of any reason not to pass it through. I'll try and take care of it in PR3 [ I'm out PR2, so don't want to risk breaking stuff just before I leave for a while ] |
|
Posted: 03/23/12
Moving out to 3.6.0 |
|
Posted: 04/4/12
|
|
Posted: 04/4/12
|
|
Posted: 05/8/12
Need to move to Sprint 2, since Sprint 1 ended up going mostly towards 3.5.1 |
|
Posted: 05/22/12
Done. Was holding off, because I thought I could take care of #2530248 at the same time, but that may need some event optimization and end up being a longer task, so getting this in now. Will close once merged to master (after I test a little more). Unit tests pass. |
|
Posted: 06/1/12
Added opts param to setAttrs(attrs, opts), to match set(attr, opts). Fixes #2531617 |
|
Posted: 08/1/12
Shipped in 3.6.0. Marking closed/fixed. |
Hey Pat,
Did it do anything in 3.4.1? Were you successfully using it?
From the code, it seemed like it didn't. I didn't pass it along, and since it was undocumented, figured it was just cruft:
/**
* Implementation behind the public setAttrs method, to set multiple attribute values.
*
* @method _setAttrs
* @protected
* @param {Object} attrs An object with attributes name/value pairs.
* @return {Object} A reference to the host object.
* @chainable
*/
_setAttrs : function(attrs, opts) {
for (var attr in attrs) {
if ( attrs.hasOwnProperty(attr) ) {
this.set(attr, attrs[attr]);
}
}
return this;
},
Trying to determine if this is a regression in master, or not. My reading of the above code is that it's not, unless it impacts a custom implementation somehow, which may be overriding _setAttrs?
In general though it's not a bad thing to add to setAttrs. I don't recall why I left it out to begin with. For the purposes of this bug though, trying to determine whether or not it's a regression from 3.4.1