431811 - Context menu of tasks in today pane is broken [Error: taskTree has no properties]

Status: VERIFIED FIXED

(Calendar :: Lightning Only, defect)

Description

[Andreas Treumann]

STEPS TO REPRODUCE:
===================

- select a task in the today pane
- open the context menu
- 1. error: Error: taskTree has no properties
Source File: chrome://calendar/content/calendar-task-tree.js
Line: 113
- move the mouse over the 'Progress', 'Priority', 'Calendar' and 'Convert to' entries
- next errors: Error: taskTree has no properties
Source File: chrome://calendar/content/calendar-task-tree.js
Line: 96

Error: taskTree has no properties
Source File: chrome://calendar/content/calendar-task-tree.js
Line: 113

Error: taskTree has no properties
Source File: chrome://calendar/content/calendar-task-tree.js
Line: 54

RESULT:
=======

- the error console shows errors

EXPECTED RESULT:
================

- no errors

REPRODUCIBLE:
=============

- always

Same on Linux, Mac and Solaris trows no errors.

Comment 1

[Stefan Sitter [:ssitter]]

Regression range:
    Works in Ltn 0.9pre (2008041720) + Tb 2.0.0.15pre (20080501).
    Fails in Ltn 0.9pre (2008041820) + Tb 2.0.0.15pre (20080501).

Checkins during regressin range: http://tinyurl.com/6bunoz
Most probably regressed by Bug 389150.

Comment 2

[Berend Cornelius]

Attachment: patch v. #1

I can't say which this issed regressed from because it's already almost weeks old.
But in bug 363932 not all calls of "modifyEventWithDialog" were adapted to the new parameters. I changed that and it works fine. On this occasion I set the default of the third parameter "aPromptOccurrence" to "false".

Comment 3

[Daniel Boelzle [:dbo]]

Comment on attachment 322512 [details] [diff] [review]
patch v. #1

r=dbo

Comment 4

[Berend Cornelius]

patch checked in on trunk and MOZILLA_1_8_BRANCH

Comment 5

[Stefan Sitter [:ssitter]]

Errors are still reproducible using Lightning 0.9pre (2008-05-26-10).

Comment 6

[Berend Cornelius]

Attachment: patch v. #2

second try: I could only confirm the error extremely sporadically (and Daniel could not confirm this at all) which is why I believe that this some kind of focus bug. This attached patch should make that behaviour more reliable. But before I ask for a a review I would appreciate it if some people could test the patch beforehand.

Comment 7

[Stefan Sitter [:ssitter]]

I don't know what platform you are developing on. According to Comment #0 this error only occurs on Windows and Linux. At least on Windows XP it happens as soon as I right click the task list in the today pane to open the context menu.

Comment 8

[Berend Cornelius]

I have noticed this and we have tested it on Linux and Mac. Have you tried it also with my new patch?

Comment 9

[Stefan Sitter [:ssitter]]

Comment on attachment 322621 [details] [diff] [review]
patch v. #2

I tried this patch on WindowsXP with Lightning 0.9pre and the errors are gone.

Comment 10

[Berend Cornelius]

The solution in my last patch is not the right solution! As we use the same task in our application menu as well as in the task context menu I can't retrieve the relevant tree based on "document.popupNode". On the other hand I dislike the idea more and more to do this focus-dependent. Probably I should attach the id of the tasktree" directly as an attribute to the application menu (e.g. "tree="calendar-task-tree") and use "document.popupNode" as a fallback for the context-menus. The remaining problem would be to remove "calendar_modify_todo_command" from calendar.commonsets.xul/js because in these files this context is not known.

Comment 11

[Berend Cornelius]

Attachment: current state of the art

not the best solution but with my modifications in "getParentNode()" this will probably also resolve the issue on all platforms. Any commment is welcome.

Comment 12

[Berend Cornelius]

Attachment: patch v. #4

Newest patch following my guidelines of comment #10. My aim was to make the commmands focus independent yet as I found out this is not so easy for the general commends for "Cut,Copy,Paste and Delete" because the always relate to the focused pane. For the latter one we still have bug 429685 – Toggling of 'Delete' - toolbarbutton in calendar-mode and task-mode does not work, that I will deal with in a follow-up. Andreas has tested this patch already and found no objections.

Comment 13

[Philipp Preißer-Kewisch [:Fallen][☁️📆]]

Comment on attachment 323053 [details] [diff] [review]
patch v. #4



>+/**
>+ * Returns a parentnode with the given attributevalue for the given attributename
>+ * by "walking up" the DOM-hierarchy.
A comment to why this function is called "OrThis" would be nice.


Also, somewhere at the top of this patch I think you called the same function by without "ByAttribute". Typo? Please ignore if thats not the case, I already deleted the other lines of this patch :-)


r=philipp

Comment 14

[Berend Cornelius]

modified the sourcecode comment  and checked in the patch on trunk and MOZILLA_1_8_BRANCH and trunk

->fixed

Comment 15

[Andreas Treumann]

Checked in nightly build 2008060118 -> VERIFIED.