475715 - Create a binding for the calendar list

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: [checked in] Fix - v1

Right now we use the calendar list in many places, each with its own implementation. I'd like to consolidate this. This patch is the first step, which moves the implementation from the main calendar list into a binding. Using this binding for other calendar lists (i.e choose calendar dialog) should be done in a different bug.

Comment 1

[Berend Cornelius]

Comment on attachment 359264 [details] [diff] [review]
[checked in] Fix - v1

Big patch. I tested it and could not detect any deficiencies. Looks like a good job. Maybe you could take yourself the time to consolidate the event handlers for the key-pressed up and key-pressed-down events.
r=berend

Comment 2

[Philipp Kewisch [:Fallen][📱📆]]

Due to limited time I'd rather leave the patch as is. It might look quite duplicated, but the common parts don't make sense on their own.

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/90197594f3b6>

-> FIXED

Comment 3

[Philipp Kewisch [:Fallen][📱📆]]

Correction: rev 43ba72c39c9f

Comment 4

[Robert]

There was a little copy&paste error in this patch. If you have a color assigned to at least one category, the color won't be displayed anymore and Thunderbird has problems to start up. A log message "aCalendar is not defined" will be created.

The fix is simple. Line 576 in /calendar/base/content/calendar-views.js says
let ruleString = '.category-color-box[categories~="' + aCalendar.id + '"] {} ';
but should say
let ruleString = '.category-color-box[categories~="' + aCatName + '"] {} ';

After changing this, everything works fine again.

Comment 5

[Stefan Sitter [:ssitter]]

(In reply to comment #4)
Maybe you could provide a patch for corresponding Bug 480933?

Comment 6

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Bustage fixes - v1

This patch takes care of all three regression bugs (bug 480383 bug 480930 bug 480933) and also bug 349987.

Comment 7

[Stefan Sitter [:ssitter]]

Comment on attachment 364991 [details] [diff] [review]
Bustage fixes - v1

Creating a new profile seems to work now but still displays the following assertion:
[[[
Error: [Exception... "Calendar list has no composite calendar yet"  nsresult: "0xc1f30001 (NS_ERROR_NOT_INITIALIZED)"  location: "JS frame :: chrome://calendar/content/widgets/calendar-list-tree.xml :: get_compositeCalendar :: line 67"  data: no] 

STACK: 
1: [file:///E:/obj/sb/mozilla/dist/bin/calendar-js/calUtils.js:1218] notifyFunc
2: [file:///E:/obj/sb/mozilla/dist/bin/calendar-js/calUtils.js:1221] calListenerBag_notify
3: [file:///E:/obj/sb/mozilla/dist/bin/calendar-js/calCalendarManager.js:574] anonymous
4: [file:///E:/obj/sb/mozilla/dist/bin/calendar-js/calCalendarManager.js:642] anonymous
5: [null:0] null
6: [chrome://calendar/content/calendar-management.js:104] initHomeCalendar
7: [chrome://calendar/content/calendar-management.js:87] loadCalendarManager
8: [chrome://calendar/content/calendar-chrome-startup.js:43] commonInitCalendar
9: [chrome://calendar/content/calendar.js:52] calendarInit
10: [chrome://sunbird/content/calendar.xul:1] onload

Source File: file:///E:/obj/sb/mozilla/dist/bin/calendar-js/calUtils.js
Line: 1218
]]]

The changes in the categories pane of the preference dialog break the following workflow:
Edit category, click the color picker and choose a color -> use color checkbox is enabled, press OK to save the changes -> changes are not saved

Changing a category color doesn't update the colors in the view for me.

The following is logged when closing the main window:
[[[
Error: categoryPrefBranch.removeObserver is not a function
Source file: chrome://calendar/content/calendar-views.js
Line: 555
]]]

Comment 8

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Bustage fixes - v2

Comment 9

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: [checked in] Bustage fixes - v3

Comment 10

[Stefan Sitter [:ssitter]]

Comment on attachment 365021 [details] [diff] [review]
[checked in] Bustage fixes - v3

I'm not sure about the eval() part but the rest looks OK. r=ssitter

Comment 11

[Stefan Sitter [:ssitter]]

Another regression that seem to be caused by this patch:
[*] Setting a calendar to read-only throws:
    Error: aPrefName is not defined
    Source File: chrome://calendar/content/calendar-task-tree.xml
    Line: 903
[*] Changing calendar color updates the events in the view with the new color but not box next to the calendar name in the list of calendars
[*] Pressing SPACE key in the calendar list doesn't toggle the visibility anymore

Comment 12

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 365021 [details] [diff] [review]
[checked in] Bustage fixes - v3

I replaced eval with a call to (new Function(arg,body))(arg)

Pushed to comm-central  <http://hg.mozilla.org/comm-central/rev/c92e3a7a0e1c>

Comment 13

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: [checked in] Second set of bustage fixes - v1

(In reply to comment #11)
> Another regression that seem to be caused by this patch:
> [*] Setting a calendar to read-only throws:
>     Error: aPrefName is not defined
>     Source File: chrome://calendar/content/calendar-task-tree.xml
>     Line: 903
Couldn't reproduce, but fixed in code.

> [*] Changing calendar color updates the events in the view with the new color
> but not box next to the calendar name in the list of calendars
This worked for me?

> [*] Pressing SPACE key in the calendar list doesn't toggle the visibility
> anymore
Fixed.

Comment 14

[Stefan Sitter [:ssitter]]

Comment on attachment 365147 [details] [diff] [review]
[checked in] Second set of bustage fixes - v1

r=ssitter but I'd add a comment to the handler like |check for key=" " because keycode="VK_SPACE" doesn't work|

The calendar color update issue still exists for me. I tested with Sunbird, maybe there is a different behavior compared to Lightning?

Comment 15

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 365147 [details] [diff] [review]
[checked in] Second set of bustage fixes - v1

I found the color change issue. It happens only when changing between colors, but not when the color was previously the default color. Fix is in the pushed patch.

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/78b9ab3d6c7e>