Lightning 1.0b2pre (BuildID: 20100422035110) with Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:18.104.22.168pre) Gecko/20100422 Lanikai/3.1b2pre Reminder "0 minutes before" is shown as "The moment the events starts" after restart Steps To Reproduce: 1. Create new Thunderbird profile and install Lightning 2. Create new event in the default calendar 3. Select "0 minutes before" from reminder dropdown menu 4. Save event and close dialog 5. Reopen the event for editing and check the reminder settings 6. Restart Thunderbird 7. Reopen the event for editing and check the reminder settings Actual results: After creation the dialog shows: Reminder: [0 minutes before |v] After restart the dialog shows: Reminder: [Custom... |v] The moment the events starts Expected results: Always show the default entry from the dropdown menu.
Solving this could be a good start for a beginner like me to contribute.. Could u please give any guide about how to get to start?
To get started, you should first get Thunderbird built with Lightning included. Se https://developer.mozilla.org/En/Simple_Thunderbird_build for details, make sure to set up your mozconfig with --enable-calendar before you start the build. The starting point for this can be found here: http://mxr.mozilla.org/comm-central/source/calendar/base/content/dialogs/calendar-dialog-utils.js#558 You need to debug through it (using printf-style debugging with cal.ERROR(), sorry) and find out why its not matching the alarm to the right menuitem. The function likely needing a fix is here: http://mxr.mozilla.org/comm-central/source/calendar/base/content/dialogs/calendar-dialog-utils.js#516
Thanks. I've built thunderbird with lightning, now I will go to work on this :)
Hi Philipp, should the expected result be that after reopen, the reminder dialog shows "no reminder"? Or showing the selection ("0 minutes before", in your description) made before the reopen? Thanks.
It should show "0 minutes before" selected in the dropdown.
Hi Fallen just wondering whether it is necessary to change this back, since "0 minute before" is the same with "the moment the event starts". (In reply to Philipp Kewisch [:Fallen] from comment #5) > It should show "0 minutes before" selected in the dropdown.
Created attachment 672303 [details] [diff] [review] add debug cal.errors, modify origin in matchCustomReminderToMenuitem() Hi Fallen I found the problem occurs when judging whether the reminder belongs to one of those in the menu list. Originally the attribute offset must be less than 0 (marked as "before") to pass the check, otherwise the reminder will be cast to a custom reminder. However "0 minute before" has an offset of 0, which makes it fail to pass the check and be cast into a custom reminder. Now I've changed how it check the offset, and allow reminder with offset not positive (include 0) to pass, and the bug seems to be fixed by this. :)
Hi Liu, I'd like to ask you to attach a proper patch. Your current patch looks fine on the first glance but adds many error/debug messages that should not be part of it.
Thank you for the advice, I think I will exclude them in the next patch :) (In reply to Stefan Sitter from comment #8) > Hi Liu, I'd like to ask you to attach a proper patch. Your current patch > looks fine on the first glance but adds many error/debug messages that > should not be part of it.
Created attachment 674660 [details] [diff] [review] The same modifications,but remove debuggers This patch can fix the bug, and the former added debuggers are removed.
Hi Fallen I found the problem is exactly at let origin = (reminder.offSet.isNegative?"before":"after"); It seems if the duration is 0(o minutes before), it is treated as not negative and hence the origin to be "after", which actually should be "before", so I was trying to use "isPositive" to include duration 0 into the "before" origin. Since we can not use isPositive, is there anything to distinguish 0 duration from positive duration?
Created attachment 675481 [details] [diff] [review] Use if-else to get the correct origin value in matchCustomReminderToMenuitem(reminder) matchCustomReminderToMenuitem(reminder) is used when load and match reminders. "0 minutes before" has no menuitem match caused by "origin = (reminders.offset.isNegative?"before":"after")", which makes its origin a "after". The patch rewrite part of the function, to get "0 minutes before" a origin of "before", and therefore get a correct menuitem match for it.
Comment on attachment 675481 [details] [diff] [review] Use if-else to get the correct origin value in matchCustomReminderToMenuitem(reminder) Review of attachment 675481 [details] [diff] [review]: ----------------------------------------------------------------- Aside from a few indentation fixes this looks fine, r=philipp I will fix them before checkin.
Pushed to comm-central changeset ed797863fdae