82710 - warnings on every url load

Status: VERIFIED FIXED

(SeaMonkey :: General, defect)

Description

[Christopher Blizzard (:blizzard)]

On every url load I'm seeing these warnings now:

An error occurred updating the cmd_cut command
An error occurred updating the cmd_copy command
An error occurred updating the cmd_paste command
An error occurred updating the cmd_selectAll command

This is a regression since yesterday.  Build is from the morning of May 25, 2001.

Comment 1

[R.K.Aa.]

after hitting back-button they appear twice

Comment 2

[kinmoz]

Cc'ing sfraser ... Simon, any clue on what's causing this? I've been seeing 
these warnings since last week sometime.

Comment 3

[Gervase Markham [:gerv]]

I seem to remember dr mentioned something like this in another bug... I could be 
way off base, though.

This error appears in:
http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/globalOver
lay.js#61

A few well-placed dump() statements should tell us exactly which bit is failing, 
and narrow the problem down a touch.

Gerv

Comment 4

[Gervase Markham [:gerv]]

The exception is raised by the line:
  enabled = controller.isCommandEnabled(command);

[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [n
sIController.isCommandEnabled]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  loca
tion: "JS frame :: chrome://global/content/globalOverlay.js :: goUpdateCommand :
: line 61"  data: no]

So, as I see it, we end up here (in nsDOMWindowController::isCommandEnabled):
http://lxr.mozilla.org/seamonkey/source/dom/src/base/nsGlobalWindow.cpp#4355

and someone who has access to the ability to build Mozilla will have to take it 
from there ;-)

Gerv

Comment 5

[edburns]

Not only that, but I get seven to nine assertions for every link traversal:

0[a32470]: ###!!! ASSERTION: NS_ENSURE_TRUE(edit) failed: 'edit', file 
D:\Projects\trunk\mozilla\dom\src\base\nsGlobalWi
ndow.cpp, line 4314
###!!! ASSERTION: NS_ENSURE_TRUE(edit) failed: 'edit', file 
D:\Projects\trunk\mozilla\dom\src\base\nsGlobalWindow.cpp, l
ine 4314
0[a32470]: ###!!! ASSERTION: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: '(!((rv) 
& 0x80000000))', file D:\Projects\trunk\m
ozilla\dom\src\base\nsGlobalWindow.cpp, line 4365
###!!! ASSERTION: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: '(!((rv) & 
0x80000000))', file D:\Projects\trunk\mozilla\dom\
src\base\nsGlobalWindow.cpp, line 4365
An error occurred updating the cmd_cut command
0[a32470]: ###!!! ASSERTION: NS_ENSURE_TRUE(edit) failed: 'edit', file 
D:\Projects\trunk\mozilla\dom\src\base\nsGlobalWi
ndow.cpp, line 4314
###!!! ASSERTION: NS_ENSURE_TRUE(edit) failed: 'edit', file 
D:\Projects\trunk\mozilla\dom\src\base\nsGlobalWindow.cpp, l
ine 4314
0[a32470]: ###!!! ASSERTION: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: '(!((rv) 
& 0x80000000))', file D:\Projects\trunk\m
ozilla\dom\src\base\nsGlobalWindow.cpp, line 4365
###!!! ASSERTION: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: '(!((rv) & 
0x80000000))', file D:\Projects\trunk\mozilla\dom\
src\base\nsGlobalWindow.cpp, line 4365
An error occurred updating the cmd_copy command
0[a32470]: ###!!! ASSERTION: NS_ENSURE_TRUE(edit) failed: 'edit', file 
D:\Projects\trunk\mozilla\dom\src\base\nsGlobalWi
ndow.cpp, line 4314
###!!! ASSERTION: NS_ENSURE_TRUE(edit) failed: 'edit', file 
D:\Projects\trunk\mozilla\dom\src\base\nsGlobalWindow.cpp, l
ine 4314
0[a32470]: ###!!! ASSERTION: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: '(!((rv) 
& 0x80000000))', file D:\Projects\trunk\m
ozilla\dom\src\base\nsGlobalWindow.cpp, line 4365
###!!! ASSERTION: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: '(!((rv) & 
0x80000000))', file D:\Projects\trunk\mozilla\dom\
src\base\nsGlobalWindow.cpp, line 4365
An error occurred updating the cmd_paste command
0[a32470]: ###!!! ASSERTION: NS_ENSURE_TRUE(edit) failed: 'edit', file 
D:\Projects\trunk\mozilla\dom\src\base\nsGlobalWi
ndow.cpp, line 4314
###!!! ASSERTION: NS_ENSURE_TRUE(edit) failed: 'edit', file 
D:\Projects\trunk\mozilla\dom\src\base\nsGlobalWindow.cpp, l
ine 4314
0[a32470]: ###!!! ASSERTION: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: '(!((rv) 
& 0x80000000))', file D:\Projects\trunk\m
ozilla\dom\src\base\nsGlobalWindow.cpp, line 4365
###!!! ASSERTION: NS_ENSURE_TRUE(NS_SUCCEEDED(rv)) failed: '(!((rv) & 
0x80000000))', file D:\Projects\trunk\mozilla\dom\
src\base\nsGlobalWindow.cpp, line 4365

Comment 6

[Simon Fraser [no longer active]]

Is this really linux only? If so, it's likely to be linux focus foo.

Comment 7

[Simon Fraser [no longer active]]

I hear that the assertions also hit on Windows.

Comment 8

[sairuh (rarely reading bugmail)]

spoke with jag, who sez this is blocking bug 82663 [being unable to close source
window via accel+W], which is xp.

Comment 9

[sairuh (rarely reading bugmail)]

let me rephrase that --jag has a strong suspicion that this might be underlying
cause of bug 82663... when this is fixed, we shall see. :)

Comment 10

[Simon Fraser [no longer active]]

The problem here is that docShell that the nsDOMWindowController is trying to get 
a content viewer from has no content viewer. I don't know why.

Comment 11

[Alec Flett]

strangely, I get a similar error trying to delete an entry in history:
An error occurred executing the cmd_delete command

no idea if it's related, but it just started appearing in the last few days, and
history has been basically unchanged for weeks.

Comment 12

[Simon Fraser [no longer active]]

Backing out hwaara's change in nsDocShell fixes this.

Comment 13

[Simon Fraser [no longer active]]

So hwaara was acting as Adam Lock's lackey. -> adamlock

Comment 14

[saari (gone)]

That patch was a band aid over the larger problem of a difference in when the
previous page gets torn down during a load. So now the edit command controller
is screaming because it is trying to update commands for a partially torn down page.

I wanted to know why the timing of when things get torn down got changed, but no
one piped up and I haven't investigated yet.

Comment 15

[Simon Fraser [no longer active]]

Should we fix the command updating to just bail if no interface is found?

Comment 16

[Dan Rosen]

Simon: That would probably silence more potential cases than you want to silence...

We shouldn't be trying to update commands for a half-torn-down page at all --
this falls into a class of problems relating to the limits of user interaction
with half-destroyed pages, which probably needs to be thought about a bit in
general, so we have our story straight.

Comment 17

[Simon Fraser [no longer active]]

dr: why not? If the page is half-torn down, you have to prevent the user from 
doing stuff with it by disabling commands. I think commands need to be able to 
update their enabledness whatever the state of their target, even if this means 
bailing to a disabled state.

Comment 18

[Dan Rosen]

Oh, I think I misinterpreted "bail." Yeah, the commands should definitely be in
a deterministic (disabled) state in this case. We just need to be sure that we
don't give up in *just any* case where we happen not to have been able to find a
controller.

Comment 19

[chris hofmann]

which bug was adamlock trying to fix?  anyone have that handy?

Comment 20

[R.K.Aa.]

bug 82141

Comment 21

[Adam Lock]

Attachment: Patch for assertions

Comment 22

[Adam Lock]

Attachment: Proper patch for assertions

Comment 23

[Adam Lock]

First patch mistakenly contained another fix, second one is correct.

The patch works by setting the focus element to nsnull much earlier on, before 
the outgoing content viewer has been destroyed. The new patch removes the 
assertions, still works for bug 82141 and doesn't regress bug 28580 or bug 
50509.

Reviews please?

Comment 24

[Simon Fraser [no longer active]]

saari should review that.

Comment 25

[selmer (gone)]

Is there any user visible impact of taking this fix?  It's good to get rid of
assertions, but we could take the risk in m.9.2 instead...

Comment 26

[Chris Abbey]

I'll confirm that Adam's second patch works fine on Linux.

Comment 27

[David Hyatt]

We need to stop command updating from happenign when focus is suppressed.  This 
patch is going to cause a slowdown as written, since you end up doing an extra 
updating of commands in between each page load.

I would also prefer the call be placed right after the focus suppression call. 

Comment 28

[saari (gone)]

Attachment: Some changes as suggested by hyatt. Smells good on mac so far

Comment 29

[saari (gone)]

I need people to beat on this patch, paying special attention to cross page 
loads and command updating

Comment 30

[Chris McAfee]

*** Bug 82141 has been marked as a duplicate of this bug. ***

Comment 31

[Chris McAfee]

Attachment: Linux-friendly version of saari's recent patch

Comment 32

[Adam Lock]

Patch appears to work correctly on Win32. I didn't notice any problems with 
incorrectly disabled/enabled commands.

Comment 33

[David Hyatt]

sr=hyatt

Comment 34

[Chris McAfee]

embed focus problem is now bug 83201.

Comment 35

[saari (gone)]

Okay, patch checked in, fixed

Comment 36

[edburns]

I verify that I no longer see many many assertions on each page load.

Comment 37

[Tracy Walker [:tracy]]

verified any linux builds this month