Closed
Bug 402325
Opened 17 years ago
Closed 16 years ago
Disable cut and paste commands when all calendars are readonly
Categories
(Calendar :: Calendar Frontend, defect)
Calendar
Calendar Frontend
Tracking
(Not tracked)
VERIFIED
FIXED
0.8
People
(Reporter: Fallen, Assigned: Fallen)
References
Details
Attachments
(1 file, 2 obsolete files)
115.03 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
Followup from bug 379174: In addition to the new event buttons, commands such as cut and paste should be disable-when-no-writable-calendars are there. It should be taken care that a smooth solution especially with the lightning command handler is found.
Flags: wanted-calendar0.8?
Assignee | ||
Comment 1•16 years ago
|
||
Depends on the offline bug, since this also has the need for such changes. I have an almost ready patch based on the offline patch in bug 393395 that uses a command dispatcher also for sunbird.
Assignee: nobody → philipp
Depends on: 393395
Assignee | ||
Updated•16 years ago
|
Status: NEW → ASSIGNED
Comment 2•16 years ago
|
||
This bug is not important enough to warrant wanted0.8 status. We would however take a fix for this bug into 0.8, if such a fix materializes.
Flags: wanted-calendar0.8? → wanted-calendar0.8-
Assignee | ||
Comment 3•16 years ago
|
||
This patch implements a command controller also for sunbird, which takes care of disabling the needed commands. Note this patch does contain some parts of the offline patch, but none of the changes hinder operation without offline support.
Attachment #295912 -
Flags: review?(daniel.boelzle)
Assignee | ||
Comment 4•16 years ago
|
||
Oops, the last version was missing two files in the patch.
Attachment #295912 -
Attachment is obsolete: true
Attachment #295914 -
Flags: review?(daniel.boelzle)
Attachment #295912 -
Flags: review?(daniel.boelzle)
Comment 5•16 years ago
|
||
Comment on attachment 295914 [details] [diff] [review] Command Controller also for Sunbird -v2 >Index: calendar/sunbird/base/content/calendar-menubar.inc >=================================================================== > <menuseparator id="before-Unifinder-Section"/> > <menuitem id="calendar-show-unifinder-menu" > type="checkbox" > label="&showUnifinderCmd.label;" >- command="calendar_show_unifinder_command" >+ observes="calendar_show_unifinder_command" > accesskey="&showUnifinderCmd.accesskey;" > checked="true"/> What is the purpose of this change and the other <command> -> <observes> transformations? Additionally if you change this for the show-unifinder-command here, you need to change this for the "Find events" toolbarbutton in calendar.xul (Sunbird) and messenger-overlay-toolbar.xul (Lightning) as well. Same goes for the corresponding menuitem in Lightning's messenger-overlay-sidebar.xul.
Assignee | ||
Comment 6•16 years ago
|
||
(In reply to comment #5) > (From update of attachment 295914 [details] [diff] [review]) > >Index: calendar/sunbird/base/content/calendar-menubar.inc > >=================================================================== > > <menuseparator id="before-Unifinder-Section"/> > > <menuitem id="calendar-show-unifinder-menu" > > type="checkbox" > > label="&showUnifinderCmd.label;" > >- command="calendar_show_unifinder_command" > >+ observes="calendar_show_unifinder_command" > > accesskey="&showUnifinderCmd.accesskey;" > > checked="true"/> > > What is the purpose of this change and the other <command> -> <observes> > transformations? > > Additionally if you change this for the show-unifinder-command here, you need > to change this for the "Find events" toolbarbutton in calendar.xul (Sunbird) > and messenger-overlay-toolbar.xul (Lightning) as well. Same goes for the > corresponding menuitem in Lightning's messenger-overlay-sidebar.xul. > I didn't really mean to change that, but in any case, observes and command do the same thing on a menuitem, but observes is much more general. Since I did not change the command string itself, it should be fine also for the toolbar.
Comment 7•16 years ago
|
||
Philipp, my experience during preparing the patch for bug 402177 was that observes and command do not always do the same thing. So if you change this, change it in all places and please test this thoroughly. However, I would prefer to keep such "cleanup changes" out of this patch (it is large enough already) and perform these changes in a followup bug. The same goes for the whitespace changes in your patch. Since I'm not a reviewer for these code areas and have therefore no final authority here, all of this is just a recommendation.
Assignee | ||
Comment 8•16 years ago
|
||
I'm totally fine with keeping the command/observes the way it was before. I initially changed many of those and took that back before uploading the patch, the lines you mentioned I must have missed. Regarding whitespace changes, I recall a decision, that these changes should NOT be done in one extra patch, since that would totally destroy cvsblame. This is why I do whitespace changes scattered across my patches. This may also destroy cvsblame, but not as extensive as it would if it were done in one patch. Also, I think there are more important things to do than fix style. Most changes are side-effects of my scripts to automatically clean up whitespaces.
Comment 9•16 years ago
|
||
Comment on attachment 295914 [details] [diff] [review] Command Controller also for Sunbird -v2 >+ /** >+ * Gives the number of registered calendars that require network access. >+ */ >+ readonly attribute PRUint32 remoteCalendars; call this NetworkCalendarCount >+ /*** >+ * Gives the number of registered readonly calendars. >+ */ >+ readonly attribute PRUint32 readonlyCalendars; call this ReadOnlyCalendarCount >+ /** >+ * Gives the number of registered calendars >+ */ >+ readonly attribute PRUint32 registeredCalendars; call this CalendarCount and adopt code and variables to follow this naming. >+ // Set up statistics >+ if (calendar.getProperty("isRemote")) { call this requiresNetwork (defaults to true if undefined) and document this at calICalendar::getProperty. Implement for storage provider (false) and ics provider (uri.scheme != "file"). other than that I hope we won't run into regressions; r=dbo
Attachment #295914 -
Flags: review?(daniel.boelzle) → review+
Assignee | ||
Comment 10•16 years ago
|
||
I'm going to check in this patch. I'll be ready for regressions tomorrow.
Attachment #295914 -
Attachment is obsolete: true
Attachment #296594 -
Flags: review+
Assignee | ||
Comment 11•16 years ago
|
||
Checked in on HEAD and MOZILLA_1_8_BRANCH -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
No longer depends on: 393395
Flags: wanted-calendar0.8-
Resolution: --- → FIXED
Target Milestone: --- → 0.8
Comment 12•16 years ago
|
||
Verified with Lightning 2008012900 and Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:1.8.1.12pre) Gecko/20080127 Calendar/0.8pre
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•