293766 - wire up undo/redo in lightning for calendar activities

Status: VERIFIED DUPLICATE of bug 369084

(Calendar :: Lightning Only, defect)

Description

[Mike Shaver (:shaver emeritus)]

mvl landed the undo/redo stuff in the tree, and we should find a way to hook it
up to the tbird undo queue, I _guess_.  Part of me wants a separate undo queue
for "calendar mode" (bug 293192), but I'm not yet sure what the best user
experience there will be.

Comment 1

[Mike Shaver (:shaver emeritus)]

(We'll probably end up copying and pasting from calendar.js into base/content,
so that we can use it in Lightning, and Sunbird can switch to that version if
they so choose; including calendar.js in our chrome is not really on.)

Comment 2

[Joey Minta]

Attachment: first attempt

First run at getting this all wired up.  Under this method, we maintain 2
separate undo/redo stacks, but they're both controlled by the same menuitem. 
If a calendar-view is being shown, then Undo/Redo work from the lightning
stack, otherwise it works from the tb-stack.  (The tb transaction manager is
fairly hardwired, plugging diretly into it looks very tough, expecially because
of m_txnType)  Thunderbird makes its Undo/Redo labels fairly descriptive, as
long as we eventually do the same (this patch doesn't), I think the user
experience would be decent.

The transaction prototype is copied directly from calendar.js, but as comment#1
says, it may be better to put this in a common place for both lightning and
Sunbird to use.  Suggestions as to where?  I'm not sure I like what moving it
does for code-flow though, but it might be ok.	Everything else is pretty
specific to lightning.

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 197206 [details] [diff] [review]
first attempt

After discussion with Joey, the plan is to move the transaction manager stuff
to base\content\common.js for sharing.

Comment 4

[Joey Minta]

Attachment: version 2

As before, with the slight modification of using Sunbird's calTransaction
explicitly.  To do this, we move it to calendarUtils.js, since both programs
use this file.

Comment 5

[Mike Shaver (:shaver emeritus)]

Comment on attachment 199179 [details] [diff] [review]
version 2

I'm not going to get to this before Nov, but dmose  should feel free to double-stamp.  I'd have to reconstruct a lot of lightning context anyway, sadly. =/

Comment 6

[Mike Shaver (:shaver emeritus)]

Comment on attachment 199179 [details] [diff] [review]
version 2

I'm not going to have time to get to this in the foreseeable future, so I'm removing the request of me.  Sorry, I'd love to, but I don't want to mis-set expectations.

(Just ask afri how much that sucks!)

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 199179 [details] [diff] [review]
version 2

After discussion with Joey, we've decided this is going to need to change some in order to make for a more intuitive experience for the user.  Removing the review request.

Comment 8

[David :Bienvenu]

Attachment: cleanup mailnews undo/redo objects so we can work with extension undo/redo objects

The initial idea of this patch was to stop casting our nsITransactions to nsMsgTxn in nsMessenger, so that extensions like Lightning can put their own nsITransactions in the undo/redo queue and have them played back. The problem was that we need to store a transaction type. So I made nsMsgTxn implement nsIPropertyBag so it could have arbitrary properties, which might be very useful in the future.  Along the way, I had to clean up the undo code a little; it was abusing nsCOMPtrs, and initializing stuff in the constructor that could fail.

There's more we can do here - for example, I would think lightning would want to alter the undo menu item to say what's being undone (e.g., undo delete task). We might make that a property of the undo object and then extensions get that for free, as long as they set the appropriate property on the undo object. 

However, I'm not convinced that Lightning really wants to share our queue - it would give a tighter integration, but maybe users want more context sensitive menu's - i.e, when they're using lightning, see lightning undo/redo actions, and when they're in a mail folder, mail undo/redo actions.

Comment 9

[David :Bienvenu]

I've checked in the mailnews patch that should help enable this capability. I'll land it on the 1.8.1 branch as well

Comment 10

[:Gavin Sharp [email: [e-mail]]]

The mailnews patch from this bug added an semi-colon to mailnews/base/util/nsMsgTxn.cpp where it shouldn't have, which caused bustage on nye. I checked in a fix.

Comment 11

[David :Bienvenu]

yes, thx for doing that, Gavin. I fixed it before landing on the 1.8.1 branch

Comment 12

[Simon Paquet [:sipaq]]

Fixed by checkin for bug 369084.