237834 - undo is able to dirty editor when command should be disabled (also redo)

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Kathleen :Brade]

The undo command in composer has a bug.  If it is called when CanUndo returns
false, it does do some things which cause the editor to be marked "dirty."

To see this in Composer:
 * open a new composer window
 * press accel-Z (note: menu command is disabled but keybindings still trigger)
 * close window
Observe that you are prompted to save.
Expectation: window should close with no prompting.

Comment 1

[Kathleen :Brade]

note: I believe the fix should go in nsEditor::Undo(PRUint32 aCount)

Comment 2

[Kathleen :Brade]

Attachment: checks for CanUndo and CanRedo

Comment 3

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 144240 [details] [diff] [review]
checks for CanUndo and CanRedo

r=[e-mail]

Comment 4

[kinmoz]

Comment on attachment 144240 [details] [diff] [review]
checks for CanUndo and CanRedo

sr=[e-mail]

Is there a reason why Redo() doesn't also call ForceCompositionEnd()?

Also, there is no longer a need to check:

   if ((nsITransactionManager *)nsnull!=mTxnMgr.get())


since that was already done in the Can*() methods, and you already have the
result in |hasTxnMgr|. If you decide to switch to using |hasTxnMgr|, don't
forget to init it. ;-)

Comment 5

[Kathleen :Brade]

Redo doesn't need to do the IME ForceCompositionEnd call because the previous
action wasn't an IME action (it would've been Undo).  I did test various IME
actions and didn't find a problem.

I decided not to change the core part of Undo and Redo because it is too close
to the end of the cycle and I didn't want to risk a regression.

For the record, checking hasTxnMgr wouldn't be needed because you can't "hasTxn"
without a transaction manager but it could be checked if we wanted to be overly
paranoid that CanUndo/CanRedo were somehow seriously broken.

Comment 6

[Asa Dotzler [:asa]]

Comment on attachment 144240 [details] [diff] [review]
checks for CanUndo and CanRedo

a=asa (on behalf of drivers) for checkin to 1.7

Comment 7

[Kathleen :Brade]

fix landed