Closed
Bug 392584
Opened 17 years ago
Closed 17 years ago
Calendar Mode Menu cleanup needed
Categories
(Calendar :: General, defect)
Calendar
General
Tracking
(Not tracked)
VERIFIED
FIXED
0.8
People
(Reporter: chris.j.bugzilla, Assigned: berend.cornelius09)
References
Details
(Whiteboard: [l10n impact][roadmap 0.8])
Attachments
(2 files, 8 obsolete files)
47.07 KB,
patch
|
michael.buettner
:
review+
chris.j.bugzilla
:
ui-review+
|
Details | Diff | Splinter Review |
2.95 KB,
patch
|
michael.buettner
:
review+
|
Details | Diff | Splinter Review |
The current (0.7.9.2007081700) calendar mode menu is misleading. Nearly all menus apply to the Mail mode but not to the calendar mode itself. As discussed in the mailing list [1] a massive restructuring is welcome [2] gives a commented overview about the proposed changes [3] points to a xul based prototype [1] http://groups.google.com/group/mozilla.dev.apps.calendar/browse_thread/thread/b696a66717b96b81/e8136e2276808905#e8136e2276808905 [2] http://spreadsheets.google.com/pub?key=plCAueWeXt4hKv3-hjAhmeg [3] http://www.cjansen.com/lightning/calendar-mode-menu.xul
Flags: blocking-calendar0.7?
Updated•17 years ago
|
Flags: blocking-calendar0.7? → blocking-calendar0.7+
Summary: Calendar Mode Menus is misleading → Calendar Mode Menu cleanup needed
Updated•17 years ago
|
Whiteboard: [l10n impact]
Assignee | ||
Comment 1•17 years ago
|
||
As I will be on vacation for a week from Thursday on I would like to publish the current state of the implementation. Basically I added and hided menu-elements as specified and applied action commands to some of the menu-items. For some menu-items there are not yet any implemented actions available and for other menu-items there are only implementations available for Sunbird (eg. "File/Open Calendar File". We have to see what can be implemented for the 0.7 train (low hanging fruits). Menus and menuitems with no action implementation will be commented out in the final patch.
Assignee | ||
Comment 2•17 years ago
|
||
As the localization process is going to start in the week to come it is necessary to check in the resources prior to the final fix of this issue. In this respect I would ask Christian and Mickey to review the patch attached that is extracted from the current implementation and only contains the .dtd files where the new strings are defined. I hope it's Ok when I will not set up formal review flags for this.
Comment 3•17 years ago
|
||
Comment on attachment 278642 [details] [diff] [review] patch containing new resources for calendar menu >+<!ENTITY calendar.releaseCmd.label "Calendar Release Notes"> You should probably replace the "Calendar" part here with "&brandShortName;".
Comment 4•17 years ago
|
||
Comment on attachment 278642 [details] [diff] [review] patch containing new resources for calendar menu >-<!ENTITY event.menu.title "Event"> >-<!ENTITY eventMenuCmd.accesskey "v"> >+<!ENTITY event.menu.label "Event"> >+<!ENTITY event.menu.accesskey "E"> Where's this about to be used? I didn't find any reference in the existing codebase nor in Christian's specification. Also, if this should be needed somewhere, where's the corrosponding 'Task' string? >+<!ENTITY calendar.importcalendar.label "Import Calendar…"> >+<!ENTITY calendar.importcalendar.key "I"> >+<!ENTITY calendar.importcalendar.accesskey "I"> What's the 'key' used for? Superfluous? >+<!ENTITY calendar.delete.label "Delete"> >+<!ENTITY calendar.delete.accesskey "D"> >+ >+<!ENTITY showCalendarlist.label "Calendar List"> >+<!ENTITY showCalendarlist.accesskey "C"> Please stick to the prevalent style -> show.calendar.list or something similar. >+<!ENTITY showCurrentView.label "Current View"> >+<!ENTITY showCurrentView.accesskey "V"> Same comment applies here... >+<!ENTITY calendar.releaseCmd.label "Calendar Release Notes"> >+<!ENTITY calendar.releaseCmd.accesskey "N"> Keep identifiers lowercase and use the brand placeholder (I already mentioned this yesterday) as Simon suggested. >+<!ENTITY task.menu.label "Task"> >+<!ENTITY task.menu.accesskey "T"> >+ >+<!ENTITY day.menu.label "Day"> >+<!ENTITY day.menu.accesskey "D"> >+ >+<!ENTITY week.menu.label "Week"> >+<!ENTITY week.menu.accesskey "W"> >+ >+<!ENTITY month.menu.label "Month"> >+<!ENTITY month.menu.accesskey "M"> >+ >+<!ENTITY next.menu.label "Next"> >+<!ENTITY next.menu.accesskey "N"> >+ >+<!ENTITY previous.menu.label "Previous"> >+<!ENTITY previous.menu.accesskey "P"> >+ >+<!ENTITY calendar.subscribeshort.label "Subscribe…"> >+<!ENTITY calendar.subscribeshort.accesskey "S"> Short? Is this needed? >+<!ENTITY calendar.unsubscribeshort.label "Unsubscribe…"> >+<!ENTITY calendar.unsubscribeshort.accesskey "U"> Short? Is this needed? >+<!ENTITY calendar.properties.label "Calendar Properties…"> >+<!ENTITY calendar.properties.accesskey "C"> >+ >+<!ENTITY calendar.showTasks.label "Show Tasks…"> >+<!ENTITY calendar.showTasks.accesskey "T"> Lowercase. >+<!ENTITY series.selectpastEvents.label "Past Events of This Series"> >+<!ENTITY series.selectpastEvents.accesskey "P"> Lowercase. >+<!ENTITY series.selectfutureEvents.label "Future Events of This Series"> Lowercase. >+<!ENTITY series.selectfutureEvents.accesskey "F"> Lowercase. >+<!ENTITY series.selectallEvents.label "All Events of This Series"> Lowercase. >+<!ENTITY series.selectallEvents.accesskey "S"> Lowercase. >+<!ENTITY calendar.selectallEvents.label "All Events of This Calendar"> Lowercase. >+<!ENTITY calendar.selectallEvents.accesskey "A"> Lowercase. r=mickey with these comments being addressed.
Attachment #278642 -
Flags: review+
Assignee | ||
Comment 5•17 years ago
|
||
in reply to comment #4: > >-<!ENTITY event.menu.title "Event"> > >-<!ENTITY eventMenuCmd.accesskey "v"> > >+<!ENTITY event.menu.label "Event"> > >+<!ENTITY event.menu.accesskey "E"> > Where's this about to be used? I didn't find any reference in the existing > codebase nor in Christian's specification. Also, if this should be needed > somewhere, where's the corrosponding 'Task' string? See messenger-overlay-sidbar.xul line 317. The "Task" string is defined by 'task.menu.label' > >+<!ENTITY calendar.importcalendar.key "I"> > What's the 'key' used for? Superfluous? I copied the resource from 'calendar.import.label" where such a key was defined. My idea was that I wanted to be consistent to that resource. <!ENTITY calendar.import.label "Import…"> <!ENTITY calendar.import.key "I"> <!ENTITY calendar.import.accesskey "I"> > >+<!ENTITY calendar.subscribeshort.label "Subscribe…"> > >+<!ENTITY calendar.subscribeshort.accesskey "S"> > Short? Is this needed? > >+<!ENTITY calendar.unsubscribeshort.label "Unsubscribe…"> > >+<!ENTITY calendar.unsubscribeshort.accesskey "U"> > Short? Is this needed? See similar resource 'calendar.subscribe.label' from which I wanted to differ >>+<!ENTITY calendar.releaseCmd.label "Calendar Release Notes"> >>+<!ENTITY calendar.releaseCmd.accesskey "N"> >Keep identifiers lowercase and use the brand placeholder (I already mentioned >this yesterday) as Simon suggested. Again I wanted to be consistent to existing resources (see 'releaseCmd.label').Using the brand placeholder is certainly better > >+<!ENTITY calendar.showTasks.label "Show Tasks…"> > >+<!ENTITY calendar.showTasks.accesskey "T"> > Lowercase. Again I wanted to be consistent to similar resources in this file. I personally like this notation better than only lowercase characters. But I will change it
Assignee | ||
Comment 6•17 years ago
|
||
When adding the placeholder to the resource menuitem I ran into an ugly nameclash because "brandShortName" delivered "Thunderbird". The brand.dtd, where this placeholder is defined is only packed in for Sunbird. For the other usecases of this placeholder it was so far desired to get the brandname of "Thunderbird". Mickey and I decided to skip this menu-item for the 0.7 release and I suggest to reuse the Thunderbird menuitem "Release Notes" and add our own action behind it in calendar mode". Thinking more about this I must say I don't like the idea of having two menuitems .. + Release Notes * Calendar Release Notes .. one below the other anyway. Christian, what do you think?
Assignee | ||
Comment 7•17 years ago
|
||
I worked in the remaining objections of mickey
Attachment #278642 -
Attachment is obsolete: true
Assignee | ||
Comment 8•17 years ago
|
||
Attachment #278631 -
Attachment is obsolete: true
Comment 9•17 years ago
|
||
Berend, you checked in a patch for this bug. Please add a comment and mark the name of the patch with a postfix like "(checked in)" if the bug is not resolved yet. Had the patch review? Please carry forward reviews if you only correct the outstanding issues.
Comment 10•17 years ago
|
||
Berend, will this also fix Bug 325214?
Assignee | ||
Comment 11•17 years ago
|
||
Yes it will. Yet as Dan said in his comment we will not be able to use the same shortcut keys as in Sunbird because many of them are already used by thunderbird.
Assignee | ||
Comment 12•17 years ago
|
||
I implemented the calendar menu as far as I thought that it makes sense. Remaining tasks are described in issue 396087. Some notes about my implementation: I ran into some key-id-clashes and had to use some new keys (after consulting Christian of course): control/cmd + I for 'New Event' control/cmd + D for 'New Event' F11 is now the key for the Today-pane.
Attachment #278779 -
Attachment is obsolete: true
Attachment #278782 -
Attachment is obsolete: true
Attachment #280802 -
Flags: ui-review?(christian.jansen)
Attachment #280802 -
Flags: review?(michael.buettner)
Comment 13•17 years ago
|
||
Berend, please answer my questions in comment#9. There was a check in, there was no patch marked with "checked in", and now all old patches are marked as obsolete.
Assignee | ||
Comment 14•17 years ago
|
||
resolved a conflict because Phillip has checked in issue386479. One last note: I added the source code for the outstanding menu-elements in comments in order to ease the future finalization of the menus.
Attachment #280802 -
Attachment is obsolete: true
Attachment #280802 -
Flags: ui-review?(christian.jansen)
Attachment #280802 -
Flags: review?(michael.buettner)
Assignee | ||
Updated•17 years ago
|
Attachment #280808 -
Flags: ui-review?(christian.jansen)
Attachment #280808 -
Flags: review?(michael.buettner)
Assignee | ||
Updated•17 years ago
|
Attachment #278779 -
Attachment is obsolete: false
Updated•17 years ago
|
Attachment #278779 -
Attachment description: overworked resource patch → overworked resource patch [checked in]
Assignee | ||
Comment 15•17 years ago
|
||
in reply to comment #9: Checked in patch "overworked resource patch" on HEAD and MOZILLA_1_8_BRANCH already last week. I apologize for not having marked it as such. For further details see comment #2. As I was on vacation till end of last week I had to check in the localization relevant resources in advance. Mickey reviewed the patch (see commenf #4 ff).
Comment 16•17 years ago
|
||
First of all, the patch doesn't cleanly apply to an up-2-date tree. I really had a hard time to get this resolved. Second, the indentation is mixed up. I think for xul files it makes really sense to stick to a minimum level of strict indentation as otherwise it hurt readability. Please keep 2 spaces between tag names and put *each* attribute on a new line, while putting attributes on the same column. You don't need to follow this suggestion and I don't want to discuss this any further, it's just a recommendation. Third, I'm not sure that we want all these 'future use'-comments in the xul file. There's a reason why you commented these items out, so they shouldn't make it into the tree. You can keep a patch with these items in the spin-off bug you already filed, but please don't clutter the file with dozens of lines that are commented out. Fourth, a strict functional test reveals that there are some bits and pieces left. So here we go... 1) Edit -> Delete should read 'Delete Message', 'Delete Event' or 'Delete Task', depending on what has been currently selected. 2) View -> Toolbars -> Mode Toolbar isn't initially being checked. 3) Hiding the mail toolbar, switching to calendar, switch back to mail, makes the mail toolbar re-appear. The corresponding item is still unchecked. 4) Show completed tasks hasn't been moved to the menu (View -> Current View) 5) View menu -> 'Multi Week' has the wrong label. 6) View menu -> 'Reminders' is missing 7) View menu -> 'Reload remote calendars' is missing 8) Go menu -> Items are in wrong order (Go - Mail, Address book, Calendar). Furthermore, shouldn't this also be displayed in mail mode? 9) Calendar menu -> 'New Event' & 'New Task' have wrong labels.
Comment 17•17 years ago
|
||
Comment on attachment 280808 [details] [diff] [review] new patch after conflict resolution >+ ltnInitializeMailMenu(); >+ moveCalendarMenuElements(); >+ setMenuElementsVisible("calendar", false); >+ document.getElementById("calendar-toolbar").setAttribute("collapsed", "true") Wouldn't it be better to handle the menu related functionality in a single function instead of spreading the initialization across several calls? Just in order to preserve locality of responsibility. Furthermore, why do you always set the collapsed attribute for the calendar toolbar? >+function ltnInitializeMailMenu() { >+// "File"-menu >+ setModeAttributetillnextMenuSeparatorById("menu_newFolder", "mail"); >+ setModeAttributeById("newAccountMenuItem", "mail"); >+ setModeAttributeById("fileAttachmentMenu", "mail"); >+ setModeAttributeById("openMessageFileMenuitem", "mail"); >... Instead of polluting the global namespace with function that are called just from this function, I suggest to move those functions inside this one as local anonymous function. Furthermore, I find it much more readable like this: [ "example-menu-item-1", "example-menu-item-2", ... "example-menu-item-n" ].forEach( function(element, index, array) { ...do something with this element... } ); >+function cloneMailMenuElement(elementId, parentNode, refNodeId) { >+ var copyMenuItem = document.getElementById(elementId).cloneNode(false); >+ if (refNodeId != null) { >+ var refNode = document.getElementById(refNodeId); >+ parentNode.insertBefore(copyMenuItem, refNode); >+ } >+ else { >+ parentNode.appendChild(copyMenuItem); >+ } >+ return copyMenuItem; >+} As this creates exact clones, you're introducing elements with same identifiers to the tree. This doesn't should be the case. >+function setModeAttributetillnextMenuSeparatorById(firstElementId, modeValue) { >+ var firstElement = document.getElementById(firstElementId); >+ if (firstElement) { >+ setModeAttributetillnextMenuSeparator(firstElement, modeValue); >+ } >+} I can't resist, please you *mixed case* for function names. I'm shuddering when forced to see something like this... :-) >+function setModeAttributetillnextMenuSeparator(element, modeValue) { >+ var bleaveloop = false; >+ while (!bleaveloop) { >+ setModeAttribute(element, modeValue); >+ bleaveloop = element.localName == "menuseparator"; >+ if (!bleaveloop) { >+ element = element.nextSibling; >+ bleaveloop = (element == null); >+ } >+ } >+} do { setModeAttribute(element, modeValue); if (element.nextSibling && element.nextSibling.localName == "menuseparator") return; } while(element = element.nextSibling); Much more readable, and think of *mixed case*... >+function setModeAttribute(element, modevalue) { >+ if (!element.hasAttribute("mode")) { >+ element.setAttribute("mode", modevalue); >+ } >+} Indentation is wrong (same applies to other functions, as well). And something like this should be a local function. >+function setModeAttributeById(elementId, modevalue) { >+ var element = document.getElementById(elementId); >+ if (element) { >+ element.setAttribute("mode", modevalue); >+ } >+} Same here... >+<menubar id="mail-menubar"> >+<menubar id="mail-menubar"> You're hooking twice into this. Please also remove the 'future' comments, it would make this file much more readable. >+try { >+ if (menu_new_init != null) { >+ var gmenu_new_init = menu_new_init; >+ } >+} catch(ex) {} >+ >+// Some menuitems have to be explicitly hided on popupshowing in Calendar mode >+// because Thunderbird makes them visible >+menu_new_init = function() { >+ gmenu_new_init(); >+ if (gCurrentMode == "calendar") { >+ var element = document.getElementById("menu_newFolder"); >+ element.setAttribute("hidden", true); >+ element = document.getElementById("menu_newVirtualFolder") >+ element.setAttribute("hidden", true); >+ } >+} Better move the new function also inside the try-catch block, as it's not necessary to install it if the above statement throws. Also, I find it more readable to write stuff like that: document.getElementById("menu_newFolder") .setAttribute("hidden", true);
Comment 18•17 years ago
|
||
This is an up-2-date patch that cleanly applies as the previously posted patch was horribly broken... :-)
Assignee | ||
Comment 19•17 years ago
|
||
In reply to comment #16: Most of your comments have already been addressed in the follow-up bug 396087. > 5) View menu -> 'Multi Week' has the wrong label. I used an existing resource "Multiweek". I suggest to change this in a later bug too > 8) Go menu -> Items are in wrong order (Go - Mail, Address book, Calendar). This was done on purpose in agreement with Christian. The Address book menuitem does not refer to a new mode, it just opens a new window and therefor I changed the order of the menuitems and also the types. > Furthermore, shouldn't this also be displayed in mail mode? > 9) Calendar menu -> 'New Event' & 'New Task' have wrong labels. I will fix this. There is a "New Event" resource available"
Reporter | ||
Comment 20•17 years ago
|
||
Comment on attachment 280808 [details] [diff] [review] new patch after conflict resolution I would like to give this an ui+, but I can't. If I apply the patch on Mac some menus occurs twice (eg. GO), or are visible in calendar & mail mode - e.g. (Message & Calendar). Some menus seam to be incomplete, but this is -- I'm guessing here-- "only" a Mac problem. Please, try to fix these bugs first. Thank you.
Attachment #280808 -
Flags: ui-review?(christian.jansen) → ui-review-
Updated•17 years ago
|
Status: NEW → ASSIGNED
Comment 21•17 years ago
|
||
We've decided to not take this for 0.7, because there are some unresolved issues on the Mac platform, which will take some time to fix. Expect this in 0.9!
Flags: blocking-calendar0.7+ → blocking-calendar0.7-
Target Milestone: 0.7 → 0.9
Comment 22•17 years ago
|
||
Comment on attachment 280808 [details] [diff] [review] new patch after conflict resolution r- based on previous comments.
Attachment #280808 -
Flags: review?(michael.buettner) → review-
Comment 23•17 years ago
|
||
Hi. I'd like to mention an accesskey problem in the Lightning calendar view context menu, happening in 0.7RC2: the item "Novo evento..." ("New event...") has accesskey "I", despite the calendar.dtd entities point otherwise: 189 <!ENTITY calendar.context.newevent.label "Novo evento…"> 190 <!ENTITY calendar.context.newevent.accesskey "N"> http://mxr.mozilla.org/l10n-mozilla1.8/source/pt-BR/calendar/chrome/calendar/calendar.dtd#189 (Or it should be menuOverlay.dtd, but I found no accesskey for this item there) I'm the localizer for pt-BR. I'm puzzled with the accesskey not corresponding the translated file. Thanks
Comment 24•17 years ago
|
||
Also, there are some items lacking accesskeys: Lightning options (Event/Task/Calendar) in Thunderbird File/New menu: http://mxr.mozilla.org/mozilla1.8/source/calendar/locales/en-US/chrome/lightning/lightning.dtd#49 49 <!ENTITY lightning.menupopup.new.event.label "Event…"> 50 <!ENTITY lightning.menupopup.new.task.label "Task…"> 51 <!ENTITY lightning.menupopup.new.calendar.label "Calendar…"> Sunbird Help menu, item "Contents": http://mxr.mozilla.org/mozilla1.8/source/calendar/locales/en-US/chrome/calendar/calendar.dtd#279 279 <!ENTITY calendar.help.label "Help Contents"> Thanks
Comment 25•17 years ago
|
||
(In reply to comment #23) Please either file a new bug or add your comments to Bug 340025 to ensure that this issues will be fixed during the context menu cleanup. (In reply to comment #24) The patch attached in this bug adds accesskeys as already written in Bug 400444 Comment #1.
Comment 26•17 years ago
|
||
Moved comment #23 to Bug 340025 Stefan, could you pls close Bug 400444? I'd do it, but don't know the correct status to use. Bug 400444 addresses the File/New menu issue I mentioned in comment #24, but not the Help Contents one. Do you know if the patch attached creates an accesskey for Contents item under Sunbird's Help menu? Many thanks, Emerson
Assignee | ||
Comment 27•17 years ago
|
||
I completely overworked my recent patches because of the visibility problems under MacOSX. The patch attached follows a new strategy in such a way that now the complete menupopups are exchanged when moving from mail mode to calendar mode and vice versa. I tested the patch under three platforms and found it was Ok. Please see also my remarks in comment #16 and consider the follow-up issues under bug 396087. The menuitem File/open/Calendar File is still disabled. The functionality for this is available for Sunbird so some consolidation work needs to be done beforehand. Unlike christians original idea I inserted an Open-Menu here - of cource after consultation with Christian. Similarily the menuitems to subscribe a calendar are disabled too. I hope all issues objected by mickey are addressed in my new patch...
Attachment #278779 -
Attachment is obsolete: true
Attachment #280808 -
Attachment is obsolete: true
Attachment #280882 -
Attachment is obsolete: true
Attachment #286277 -
Flags: ui-review?(christian.jansen)
Attachment #286277 -
Flags: review?(michael.buettner)
Updated•17 years ago
|
Flags: wanted-calendar0.8?
Target Milestone: 0.9 → 0.8
Comment 29•17 years ago
|
||
Comment on attachment 286277 [details] [diff] [review] patch v.4 >+#ifdef XP_UNIX >+ removeCalendarMenuElementById(menupopup, "menu_find"); >+#else >+ removeMenuElementsInSectionById(menupopup, "menu_find"); I think it's better to inspect the contents and decide on the action to take instead of relying on the preprocessor. >+ for (var i = 0; i < menulist.length; i++) { >+ var menu = menulist[i]; >+ var oldmenupopup = menu.firstChild; >+ if (oldmenupopup) { >+ var newmenupopup = null; >+ var oldmenupopuplist = null; >+ var newmenupopuplist = null; >+ if (gCurrentMode == "mail") { >+ oldmenupopuplist = calendarpopuplist; >+ newmenupopuplist = mailpopuplist; >+ } >+ else if (gCurrentMode == "calendar") { >+ oldmenupopuplist = mailpopuplist; >+ newmenupopuplist = calendarpopuplist; >+ } >+ oldmenupopuplist[i] = oldmenupopup.cloneNode(true); >+ menu.replaceChild(newmenupopuplist[i].cloneNode(true), oldmenupopup); cloneNode() here is not necessary. You just need to swap the references to the menupopup elements, no need to clone each time we switch the mode. Furthermore, I'm wondering why ltnInitializeMailMenu() still calls a local function for a number of elements. I made a suggestion in my previous review comment: [ "example-menu-item-1", "example-menu-item-2", ... "example-menu-item-n" ].forEach( function(element, index, array) { ...do something with this element... } ); r- based on the unnecessary cloneNode() stuff.
Attachment #286277 -
Flags: review?(michael.buettner) → review-
Assignee | ||
Updated•17 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•17 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•17 years ago
|
Flags: wanted-calendar0.8? → wanted-calendar0.8+
Whiteboard: [l10n impact] → [l10n impact][roadmap 0.8]
Updated•17 years ago
|
Status: REOPENED → NEW
Assignee | ||
Comment 30•17 years ago
|
||
I worked in the remaining issues as far as I could. Note that I have not yet considered the taskmode.
Attachment #286277 -
Attachment is obsolete: true
Attachment #290236 -
Flags: ui-review?(christian.jansen)
Attachment #290236 -
Flags: review?(michael.buettner)
Attachment #286277 -
Flags: ui-review?(christian.jansen)
Comment 31•17 years ago
|
||
Comment on attachment 290236 [details] [diff] [review] patch v. 5 r=mickey.
Attachment #290236 -
Flags: review?(michael.buettner) → review+
Reporter | ||
Comment 32•17 years ago
|
||
Comment on attachment 290236 [details] [diff] [review] patch v. 5 Awesome patch. Tested on Tiger. Nothing more to add. Thanks Berend. ui+
Attachment #290236 -
Flags: ui-review?(christian.jansen) → ui-review+
Comment 33•17 years ago
|
||
Remember to block or invalidate bug 386497, because this patch includes an equivalent for attachment 289763 [details] [diff] [review].
Assignee | ||
Comment 34•17 years ago
|
||
patch checked in on trunk and MOZILLA_1_8_BRANCH
Status: NEW → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
Comment 35•17 years ago
|
||
1) view -> toolbar -> uncheck mail toolbar 2) switch to mail mode 3) switch back to calendar mode the mail toolbar will be displayed (but it shouldn't), but the option is still disabled. this should have been fixed with this patch...
Updated•17 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 36•17 years ago
|
||
As this patch didn't solve the previously mentioned problem where Bug 386497 specifically addresses this issue, I'm going to change the dependencies accordingly.
Depends on: 386497
Assignee | ||
Comment 37•17 years ago
|
||
This patch should resolve the remaining issue with the checked menuitem mentioned in comment #35. It also addresses a problem that can be reproduced as follows: - switch from mail-mode to calendar mode - switch to task-mode - switch to calendar-mode - switch to mailmode -> an error occurs and the switch fails.
Attachment #291907 -
Flags: review?(michael.buettner)
Comment 38•17 years ago
|
||
Comment on attachment 291907 [details] [diff] [review] patch v. 6 A patch that fixes two problems by just removing code, cool ;-) r=mickey.
Attachment #291907 -
Flags: review?(michael.buettner) → review+
Assignee | ||
Comment 39•17 years ago
|
||
patch checked in on trunk and MOZILLA_1_8_BRANCH
Status: REOPENED → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Status: RESOLVED → VERIFIED
Updated•16 years ago
|
Flags: wanted-calendar0.8+
Flags: blocking-calendar0.7-
You need to log in
before you can comment on or make changes to this bug.
Description
•