Closed Bug 337885 Opened 14 years ago Closed 14 years ago
Many xbl <setter>s don't return val
All the setters in Calendar: http://landfill.mozilla.org/mxr-test/mozilla/search?string=%3Csetter%3E&find=calendar&filter=
The bugspam monkeys have struck again. They are currently chewing on default assignees for Calendar. Be afraid for your sanity!
Assignee: base → nobody
Added "return val;"s to setter method. Pay special attention to <http://landfill.mozilla.org/mxr-test/mozilla/source/calendar/base/content/calendar-month-view.xml#601> (that is marked with a comment in the patch.) I wasn't sure what to do about it, so I just left it as it is now (returning this.mIndex).
Assignee: nobody → ehsan.akhgari
Status: NEW → ASSIGNED
Attachment #235864 - Flags: first-review?(jminta)
Comment on attachment 235864 [details] [diff] [review] Setter return val patch matt, can you take a first pass at this?
Comment on attachment 235864 [details] [diff] [review] Setter return val patch >Index: calendar/base/content/calendar-month-view.xml >=================================================================== >@@ -594,16 +596,17 @@ > <setter><![CDATA[ > this.mIndex = val % 7; > > var label = document.getAnonymousElementByAttribute(this, "anonid", "label"); > var dayName = calGetString("dateFormat", "day." + (this.mIndex+1) + ".name"); > label.setAttribute("value", dayName); > > return this.mIndex; >+ /* should this return this.mIndex directly? */ > ]]></setter> I think this is fine to leave alone. It's returning val % 7, and since we're already returning a value, we may be using it elsewhere. Otherwise, r1=lilmatt. Nice job.
Attachment #235864 - Flags: first-review?(mattwillis) → first-review+
Comment on attachment 235864 [details] [diff] [review] Setter return val patch This looks great. I agree that returning this.mIndex is fine, since it's just a number. r2=jminta Thanks for the patch!
Attachment #235864 - Flags: second-review?(jminta) → second-review+
Patch checked in on MOZILLA_1_8_BRANCH and trunk. -> FIXED Thanks!
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.