Follow-up from bug 411849: Investigate unconditional removal of 'disabled' state in function changeContextMenuForTask(aEvent) in calendar/base/content/calendar-task-tree.js: 111 document.getElementById("calendar_new_todo_command").removeAttribute("disabled"); 112 document.getElementById("calendar_new_todo_todaypane_command").removeAttribute("disabled");
Opening context menu on todo panel of todaypane with no tasks selected (e.g. just after Thunderbird has started), the function 'applyAttributeToMenuChildren'  called inside changeContextMenuForTask , sets all popup menu items with a "command" attribute to 'disabled' state, but 'New Task ..." item, i.e. "calendar_new_todo_command", must be enabled to allow to create a new task, so the code lines 111 and 112 in calendar-task-tree.js file remove the disabled state for new_todo_command that must be the only enabled command when no task is selected. Might it be the right answer?  http://mxr.mozilla.org/comm-central/source/calendar/base/content/calendar-task-tree.js#98  http://mxr.mozilla.org/comm-central/source/calendar/base/src/calUtils.js#1477)
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:184.108.40.206pre) Gecko/20090826 Calendar/1.0pre Doesn't seem correct, at least in the following case: All calendars are read-only or switched off. Opening the context menu enables the New Task menu entry and New Task toolbar button although it is not possible to create a new task.
Confirmed as bug per comment#2.
Created attachment 401818 [details] [diff] [review] patch (In reply to comment #2) > Doesn't seem correct, at least in the following case: All calendars are > read-only or switched off. Opening the context menu enables the New Task menu > entry and New Task toolbar button although it is not possible to create a new > task. Therefore, basically, it should be checked only the enabled state of the commands "calendar_new_todo_command" and "calendar_new_todo_todaypane_command". I tried this patch with enable/disable/read-only/writable calendars and with a calendar that doesn't support tasks (Google Calendar + Google Provider), and it seems working in every case. I'm not completely sure whether 'removeAttribute(disabled)' is the same of 'setAttribute("disabled", false)'. If not, can be used a 'if-else' statement with both 'removalAttribute' and 'setAttribute'.
Comment on attachment 401818 [details] [diff] [review] patch >+ let newTodoEnabled = calendarController.isCommandEnabled("calendar_new_todo_command") && >+ calendarController.isCommandEnabled("calendar_new_todo_todaypane_command"); >+ document.getElementById("calendar_new_todo_command").setAttribute("disabled", !newTodoEnabled); >+ document.getElementById("calendar_new_todo_todaypane_command").setAttribute("disabled", !newTodoEnabled); setAttribute doesn't take a boolean as a second argument, but a string, so you need to setAttribute/removeAttribute there. You could do setAttribute("x", "false"), but its more common to remove the attribute. Please see our helper in calendar-ui-utils.js. We have setBooleanAttribute, but I just say its using "true"/"false" too. The other possibility is setElementValue("calendar_new_todo_todaypane_command", !newTodoEnabled && "true", "disabled"); (or similar, please doublecheck). Setting the second parameter to boolean false (=== comparison) causes the attribute to be removed. codewise r+ with that changed, please upload a new patch.
Created attachment 401853 [details] [diff] [review] patch-v2 (In reply to comment #5) > setAttribute doesn't take a boolean as a second argument, but a string, so you > need to setAttribute/removeAttribute there. You could do setAttribute("x", > "false"), but its more common to remove the attribute. > Here the difference. Sorry for that and thanks for your explanation. In my defence (;-)) I have to say that before doing the patch I checked the "setAttribute" function on calendar code (by mxr) and aside from "false" and "true" arguments, I found a lot of setAttribute("disabled", true/false) or even cases with variables as second argument. I've done in the more common way by removing the attribute when it should be disabled.
Since "patch-v2" has changes request in comment #5, I don't think it needs another review.
Sorry for the late checkin, forgot about this one! Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/5b06b0538127> -> FIXED