Closed
Bug 438546
Opened 16 years ago
Closed 16 years ago
"Select All" (Ctrl+A) & "Undo" (Ctrl+Z) in Mail mode doesn't work anymore
Categories
(Calendar :: Lightning Only, defect)
Calendar
Lightning Only
Tracking
(Not tracked)
VERIFIED
FIXED
0.9
People
(Reporter: mschroeder, Assigned: berend.cornelius09)
References
Details
(Keywords: regression)
Attachments
(3 files)
650 bytes,
patch
|
berend.cornelius09
:
review+
|
Details | Diff | Splinter Review |
1.46 KB,
patch
|
Details | Diff | Splinter Review | |
1.54 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
"Select All" (Ctrl+A) in Mail mode doesn't work anymore, but unlike bug 412613 no message in error console.
Flags: blocking-calendar0.9?
Updated•16 years ago
|
Flags: blocking-calendar0.9? → blocking-calendar0.9+
Comment 1•16 years ago
|
||
Confirmed... If I type Ctrl-A in Mail mode, all of the events are selected in month view (when I switch to Calendar mode).
Comment 2•16 years ago
|
||
Perhaps related, these things also don't work anymore: 1) Ctrl-Z / "Edit > Undo" in Mail mode 2) The Delete command when right click in "Message Pane" (AKA "Preview Pane") in Mail mode. (I haven't updated Thunderbird since 2 May 2008 -- still using 2.0.0.14 -- so think it's a Lightning problem.)
Comment 3•16 years ago
|
||
So where (in the lighning.xpi tree structure) would I start looking to work on it? Is this where the keypress is handled by thunderbird and passed to lightning in a structure or would lightning be grabbing and processing all keypresses and passing back to thunderbird what it doesn't need? I posted on http://forums.mozillazine.org/viewtopic.php?f=46&t=675995&p=3471065&e=3471065 and got an answer "submit a patch". 100% accurate, but usless. Any ideas where to start looking would help. thanks.
Comment 5•16 years ago
|
||
The patch in bug 429685 probably caused these problems. In particular I think that the following code must be restored to function cC_doCommand(aCommand) in "calendar-common-sets.js": > if (this.defaultController && !this.isCalendarInForeground()) { > // If calendar is not in foreground, let the default controller take > // care. If we don't have a default controller (i.e sunbird), just > // continue. > this.defaultController.doCommand(aCommand); > return; > } I restored that code in the "default:" case of the first Switch statement and now Ctrl-A and Undo work again in Mail mode. Unfortunately that doesn't fix another problem: the delete command doesn't work in the context menu when I right click in Thunderbird's Preview Pane. I've traced this second problem to the same Switch statement. The following code that's in capital letters should execute but it doesn't: > case "calendar_delete_event_command": > case "cmd_delete": > case "button_delete": > var focusedElement = document.commandDispatcher.focusedElement; > if (focusedElement) { > var focusedRichListbox = getParentNodeOrThis(focusedElement, "richlistbox"); > if (focusedRichListbox && focusedRichListbox.id == "agenda-listbox") { > agendaListbox.deleteSelectedItem(false); > } else if (focusedElement.className == "calendar-task-tree") { > deleteToDoCommand(null, false); > } ELSE IF (THIS.DEFAULTCONTROLLER && !THIS.ISCALENDARINFOREGROUND()) { > THIS.DEFAULTCONTROLLER.DOCOMMAND(ACOMMAND); > } else { > deleteSelectedEvents(); > } > } > break;
Comment 6•16 years ago
|
||
Would you mind showing your changes to us as in uni-diff notation? It is unclear to me where exactly to change the code in that file. Maybe you could also point to a SVN/CVS changeset where we can see which other patch you are referring to. Sorry for asking for convenience here. I am used to linking comments to repositories in Trac. Is this not possible in Bugzilla?
Comment 7•16 years ago
|
||
Okay, I found the LXR service, still being a newbie and not having known about it before. You are probably referring to this changeset: http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/calendar/base/content&command=DIFF_FRAMESET&file=calendar-common-sets.js&rev2=1.1.2.7&rev1=1.1.2.6 Consequently, in order to fix Ctrl-A, the following patch needs to be applied: --- calendar-common-sets.js.ori Fri Jun 20 09:52:20 2008 +++ calendar-common-sets.js Fri Jun 20 09:48:38 2008 @@ -200,6 +200,13 @@ }, doCommand: function cC_doCommand(aCommand) { + if (this.defaultController && !this.isCalendarInForeground()) { + // If calendar is not in foreground, let the default controller take + // care. If we don't have a default controller (i.e sunbird), just + // continue. + this.defaultController.doCommand(aCommand); + return; + } switch (aCommand) { // Common Commands case "calendar_new_event_command":
Comment 8•16 years ago
|
||
Reporter | ||
Updated•16 years ago
|
Summary: "Select All" (Ctrl+A) in Mail mode doesn't work anymore → "Select All" (Ctrl+A) & "Undo" (Ctrl+Z) in Mail mode doesn't work anymore
Comment 9•16 years ago
|
||
Alexander, thanks for quickly offering a fix, I hope that the calendar developers will also give this bug their full attention. I helped as much as I could (took me several hours), but I know nothing about Mozilla's CVS so I didn't post anything in "uni-diff notation", but hoped that I found the source of the problems. Regarding your patch, I suspect that you might have to put the lost IF statement at the end of the first Switch statement in its "default:" section instead of before the Switch statement. I'm guessing this is true so that events that fire from the today pane can be properly handled even when Thunderbird is in Mail Mode. Honestly, however, I don't think you're going to make much progress until Berend has time to take ownership of this bug. In addition to the problem that you fixed, there is the second problem that I mentioned (delete in context menu is broken) that I think Berend is most qualified to investigate since it's "his baby".
Comment 10•16 years ago
|
||
It is not my fix, but yours. The credit goes to you. I know nothing about this module and just tried to find a way to apply it as a quick workaround until somebody takes care of it more thoroughly. At least it fixes my Ctrl-A problem, the delete case is not one I use a lot. I know that this patch is not the "real" fix.
Comment 11•16 years ago
|
||
Well and done, but who or when will this patch be applied to the lightning src? It still is not there as of 20080623. I've been self applying it but it needs to be blesed and incorporated into the tree.
Comment 12•16 years ago
|
||
Comment on attachment 325941 [details] [diff] [review] Ctrl-A patch according to Pete Riley's posting In order to get a patch into the tree, it needs to be reviewed. See http://wiki.mozilla.org/Calendar:Module_Ownership for a list of reviewers. In this case, best would be berend or philipp.
Attachment #325941 -
Flags: review?(Berend.Cornelius)
Assignee | ||
Comment 13•16 years ago
|
||
Comment on attachment 325941 [details] [diff] [review] Ctrl-A patch according to Pete Riley's posting The location of your patch within calendar-common-sets.js is not quite where it should be. Basically the problem is that the commands cmd_selectAll, cmd_cut, cmd_copy, cmd_pase, cmd_print are so far only implemented for the calendar view. But as we meanwhile have a todaypane in all three modes the commands should execute upon the focusedElement. This is already implemented for the delete button but not yet other mentioned commands (in fact your patch did not consider the delete-Command, that's why I had to modify it) For the commannds cmd_undo and cmd_redo I think a prior discussion is necessary how we want to handle this: Do we want to keep an undo redo queue separate from Thunderbird's queue in mail mode and in the other modes? I will set up two follow-up-bugs for this. For the time being this patch is Ok and I will check in the modified version.
Attachment #325941 -
Flags: review?(Berend.Cornelius) → review+
Assignee | ||
Comment 14•16 years ago
|
||
I submitted Bug 441308 as a follow-up to this issue and checked in the attached patch. ->fixed
Assignee | ||
Comment 15•16 years ago
|
||
I set up Bug 441316 – Undo/Redo does not work in Mail-mode for calendar-actions. I have not yet made any proposals on how realize and implement this, but maybe somebody else wants to go ahead.
Comment 16•16 years ago
|
||
Thanks for the patch, Berend, but it doesn't seem to fix the second part of the regression -- deletion is broken in Thunderbird's context menu. As I mentioned in comment 5, something is wrong with this code: > if (focusedRichListbox && focusedRichListbox.id == "agenda-listbox") { > agendaListbox.deleteSelectedItem(false); > } else if (focusedElement.className == "calendar-task-tree") { > deleteToDoCommand(null, false); > } ELSE IF (THIS.DEFAULTCONTROLLER && !THIS.ISCALENDARINFOREGROUND()) { > THIS.DEFAULTCONTROLLER.DOCOMMAND(ACOMMAND); It seems that either --> focusedRichListbox && focusedRichListbox.id == "agenda-listbox" or --> focusedElement.className == "calendar-task-tree" is always true when people right click in Thunderbird's preview pane, so the deletion event is never passed to the default controller.
Comment 17•16 years ago
|
||
Guys, I now said twice at least that the patch only fixes Ctrl-A, please do read other people's comments more thoroughly.
Assignee | ||
Comment 18•16 years ago
|
||
in reply to comment #16: Somehow I did not check the "previewpane" error. I am sorry. This is what caused that error: By the moment the user right-clicks on the previewpane no pane has the focus and that's what "my" code is not prepared for. The patch attached should fix this problem. In the future the code around this patch (where the focused element is retrieved) should be more generalized so that we can extract the "focus logic" to implement the "cut,copy,paste, print" and also the remaining "undo-redo" handling in the same way.
Assignee: nobody → Berend.Cornelius
Status: NEW → ASSIGNED
Attachment #326446 -
Flags: review?(philipp)
Updated•16 years ago
|
Attachment #326446 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 19•16 years ago
|
||
patch checked in on trunk and MOZILLA_1_8_BRANCH
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 20•16 years ago
|
||
I patched manually and tried Ctrl-Z as well as context menu -> delete. Both works, but I have not tried before having patched. I guess the fix works, though. But strange enough, Ctrl-Y (redo) does not work. Doing redo via menu works, though. Maybe another add-on is interfering, but would somebody double-check, please? I do not know if this effect is connected to the same problem, but if so, maybe there is something else which needs to be fixed.
Comment 21•16 years ago
|
||
Verified fixed in Lightning 0.9pre 2008-06-24 18 / Thunderbird 2.0.0.14 / WinXP. Thank you! :) (Alexander, I don't think that Ctrl-Y has ever worked for me in Thunderbird, so it's probably not related to this bug.)
Comment 23•16 years ago
|
||
> (Alexander, I don't think that Ctrl-Y has ever worked for me in > Thunderbird, so it's probably not related to this bug.) Bug 365810, ought to be fixed in Thunderbird 3.
Reporter | ||
Updated•16 years ago
|
Attachment #326314 -
Attachment is obsolete: true
Reporter | ||
Updated•16 years ago
|
Attachment #326314 -
Attachment is obsolete: false
You need to log in
before you can comment on or make changes to this bug.
Description
•