75271 - Edit|Undo does not highlight if table is the first thing added

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[shrirang khanzode]

seen on windows trunk 0409

steps:
Launch composer
click on the TABLE button in composition toolbar and add the 2x2 table
now select 'Edit' menu and observe that UNDO appears disabled

expected: UNDO should appear enabled

Comment 1

[rubydoo123]

this is weird, the undo will be enabled if I minimize the window and then 
redisplay the window, not really sure who this should go to

Comment 2

[rubydoo123]

charley, handing this one over to you to see if you can see anything in the 
xul/css that might be causing this

Comment 3

[shrirang khanzode]

cc:kin

Comment 4

[Charles Manske]

Sounds like another instance of command enable/disable state not updating
correctly.
Always cc Simon on bugs like this so he can see the various examples.

Comment 5

[kinmoz]

The bug here is that nsInterfaceState assumes that after a transaction is done 
(executed) that there will be something on the undo stack. This is not the case 
when batches are involved since executed transactions get added to the batch 
instead of the undo stack until the batch is closed. 

We'll need to add some mechanism to nsInterfaceState that tracks 
DidEndBatch() notifications similar to what is done in DidDo(), but also tracks 
nested batches ... in this scenario DidDo() would only set mFirstDoOfFirstUndo 
to false if the batch nest count was zero.

Comment 6

[Charles Manske]

Removing the BeginBatchChanges() and EndBatchChanges() in EdInsertTable.js
confirms what Kin described. So Kin or Joe would have to fix this?
Note that I don't see "Undo" enabled after minimizing or window resizing or
anything else that *doesn't* trigger another transaction, such as typing.

Comment 7

[kinmoz]

nsInterfaceState.cpp is sfraser's baby. Simon, I can help you out with this if 
you're overloaded for 0.9.1.

Comment 8

[Simon Fraser [no longer active]]

I'll take a look.

Comment 9

[Simon Fraser [no longer active]]

Fixing this could be quite a big performance win. Calling UpdateCommands() is 
expensive, and we're doing it a lot for batched transactions.

Comment 10

[Simon Fraser [no longer active]]

Attachment: nsInterfaceState fixes

Comment 11

[kinmoz]

sr=[e-mail]

Comment 12

[Akkana Peck]

r=akkana

Comment 13

[Simon Fraser [no longer active]]

Fix checked in.

Comment 14

[sujay]

verifiedin 4/19 build.