Closed
Bug 840913
Opened 11 years ago
Closed 11 years ago
Remove use of nsISupportsArray in Calendar's treeview implementation [Error: TypeError: aProps is undefined]
Categories
(Calendar :: Internal Components, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
2.4
People
(Reporter: Fallen, Assigned: mmecca)
References
Details
Attachments
(1 file)
11.33 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
Lightning still uses nsISupportsArray in various places. Support for this is going away soon after the merge, see bug 407956 for the nsITreeView part. Getting rid of it is fairly straightforward for the tree view.
Assignee | ||
Comment 1•11 years ago
|
||
Bug 407956 has landed, the task list is currently broken on trunk with many instances of: Error: TypeError: aProps is undefined Source file: chrome://calendar/content/calendar-task-tree.xml Line: 517
Assignee: nobody → matthew.mecca
Severity: normal → blocker
Assignee | ||
Comment 2•11 years ago
|
||
Attachment #721656 -
Flags: review?(philipp)
Reporter | ||
Comment 3•11 years ago
|
||
Comment on attachment 721656 [details] [diff] [review] Fix v1 Review of attachment 721656 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for beating me to this. I was actually working on a such patch myself just a minute ago, but I had strange errors that aProps is undefined that were driving me crazy. Did you test the patch? I had some missing icons in the calendar list on my own patch and I remember seeing a similar bug for chatzilla. r=philipp with the following comments: ::: calendar/base/content/calendar-task-tree.xml @@ +508,2 @@ > } > + return ""; This can be shortened as: return aCol.element.getAttribute("anonid") || ""; @@ +536,5 @@ > // Task categories > var categories = item.getCategories({}); > categories.map(formatStringForCSSRule) > .map(getAtomFromService) > + .forEach(properties.push, properties); The getAtomFromService line is no longer needed, otherwise you are adding atoms. I suggest: properties = properties.concat(item.getCategories({})); Then in the return line: return properties.map(formatStringForCSSRule).join(" "); and remove the formatStringForCSSRule in any other lines above. If you don't agree (i.e perf reasons, the static strings don't need formatStringForCSSRule) I'd at least use concat() here and drop the getAtomFromService() ::: calendar/base/content/calendar-unifinder.js @@ +704,5 @@ > } > > // Task categories > item.getCategories({}).map(formatStringForCSSRule) > .map(getAtomFromService) Same getAtomFromService issue ::: calendar/base/content/widgets/calendar-list-tree.xml @@ +647,2 @@ > } else { > + properties.push("unchecked"); Since the lines are now shorter, you can condense this to properties.push(composite.getCalendarById(calendar.id) ? "checked" : "unchecked"); @@ +655,5 @@ > let color = (calendar.getProperty("color") || "").substr(1); > > // Set up the calendar color (background) > let bgColorProp = "color-" + (color || "default"); > + properties.push(bgColorProp); Since the lines are now shorter, you can also drop the intermediate bgColorProp variable and push directly. Same for a few of the other props.
Attachment #721656 -
Flags: review?(philipp) → review+
Reporter | ||
Comment 4•11 years ago
|
||
(In reply to Philipp Kewisch [:Fallen] from comment #3) > Thanks for beating me to this. I was actually working on a such patch myself > just a minute ago, but I had strange errors that aProps is undefined that > were driving me crazy. > > Did you test the patch? I had some missing icons in the calendar list on my > own patch and I remember seeing a similar bug for chatzilla. Argh, the error I was getting was actually an error with one of my extensions that hooked into the calendar list tree. Your patch was actually working quite fine. Remaining review comments still stand of course.
Updated•11 years ago
|
Summary: Remove use of nsISupportsArray in Calendar's treeview implementation → Remove use of nsISupportsArray in Calendar's treeview implementation [Error: TypeError: aProps is undefined]
Assignee | ||
Comment 5•11 years ago
|
||
Pushed to comm-central - https://hg.mozilla.org/comm-central/rev/324320ba7679
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][mentor=Fallen][lang=js]
Target Milestone: --- → 2.4
You need to log in
before you can comment on or make changes to this bug.
Description
•