1446522 - Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDocShell.doCommand] when clicking appMenu-cut-button

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[magicp]

Steps to reproduce:
1. Launch Nightly (Cursor will be in urlbar)
2. Click home button (Cursor lost focus from urlbar)
3. Open hamburger menu (Cut, Copy and Paste button will be enabled)
4. Click Cut button

Actual results:
In step 4, the following error is logged in the browser console.

NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDocShell.doCommand]  browser-child.js:407


Expected results:
No error.
In step 3, cut, copy and paste button are enable even if cursor lost focus from urlbar. Is this expected?

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

Gijs, do you happen to know if there was a recent change in this code?

Comment 2

[:Gijs (he/him)]

It looks like in the past this focused the search box in the homepage. It doesn't do that anymore. This worked in 58, broken in 60 and 61. I'll see if I can find a regression window.

The buttons being enabled in that case (when, afaict, nothing editable is focused)... I don't know what that's about. Maybe Neil knows.

Comment 3

[:Gijs (he/him)]

Looks like the focus getting lost is a 'regression' from bug 1421917. Mardak, it seems like that was intentional (per bug 1446828), is that right? I'm a little surprised that's just been left broken... focus being in the "wrong" place would be OK, but focus being nowhere is pretty bad...

Hopefully Neil can tell us why the cut/copy/paste buttons are enabled when focus is lost.

Comment 4

[Ed Lee :Mardak]

Focus isn't "lost" -- it's on the page. E.g., you can hit space / down to scroll the page after clicking home.

Comment 5

[Ed Lee :Mardak]

I see the same doCommand ERROR_FAILURE on any page where you focus just the page. E.g., click an empty background space in bugzilla and click the hamburger cut button.

Comment 6

[Neil Deakin]

Do you know where the error is being generated? From nsClipboardCommand::DoCommand?

Comment 7

[:Gijs (he/him)]

(In reply to Neil Deakin from comment #6)
> Do you know where the error is being generated? From
> nsClipboardCommand::DoCommand?

Yes. Seems actionTaken is false. It also seems that in docshell::doCommand, getting the command handler already returns NS_ERROR_FAILURE, but we just null check the command handler and then use it anyway (we don't check the nsresult). Does that help? As noted in comment 5, this is reasonably easy to reproduce.

Perhaps the main question should be why we don't update the button state (we also don't update the main edit menu correctly, so it doesn't seem specific to the buttons)

Comment 8

[Neil Deakin]

Since bug 1159490, Cut/Copy/Paste are always enabled in html documents.

The code here should just not produce an error in this case, perhaps by just ignoring actionTaken and always returning success.

Comment 9

[:Gijs (he/him)]

Attachment: Bug 1446522 - don't return NS_ERROR_FAILURE for cut/copy/paste even when there's no selection,

Review commit: https://reviewboard.mozilla.org/r/231308/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/231308/

Comment 10

[:Gijs (he/him)]

Comment on attachment 8962478 [details]
Bug 1446522 - don't return NS_ERROR_FAILURE for cut/copy/paste even when there's no selection,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/231308/diff/1-2/

Comment 11

[Neil Deakin]

Comment on attachment 8962478 [details]
Bug 1446522 - don't return NS_ERROR_FAILURE for cut/copy/paste even when there's no selection,

https://reviewboard.mozilla.org/r/231308/#review237234

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/baf5f1356969
don't return NS_ERROR_FAILURE for cut/copy/paste even when there's no selection, r=enndeakin+6102

Comment 13

[Narcis Beleuzu [:NarcisB]]

Backed out for clipboard failures on /test_bug1012662_noeditor.html

Push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=baf5f13569695b9efbad29ead950218e0be027a8
Backout: https://hg.mozilla.org/integration/autoland/rev/2b7a885a0150de69d128d717e0e2dc243cb1017b
Log: https://treeherder.mozilla.org/logviewer.html#?job_id=170762064&repo=autoland&lineNumber=11013

Comment 14

[:Gijs (he/him)]

(In reply to Narcis Beleuzu [:NarcisB] from comment #13)
> Backed out for clipboard failures on /test_bug1012662_noeditor.html
> 
> Push:
> https://treeherder.mozilla.org/#/
> jobs?repo=autoland&revision=baf5f13569695b9efbad29ead950218e0be027a8
> Backout:
> https://hg.mozilla.org/integration/autoland/rev/
> 2b7a885a0150de69d128d717e0e2dc243cb1017b
> Log:
> https://treeherder.mozilla.org/logviewer.
> html#?job_id=170762064&repo=autoland&lineNumber=11013

It look like this test fails because we expect the `document.execCommand` call to return false when no clipboard operation takes place. What's an appropriate point in the callchain to make this distinction? That is, I assume I have to catch the error somewhere else somehow... but it's not clear to me where. Specialcasing the clipboard command elsewhere seems hacky...

Comment 15

[Neil Deakin]

I suppose the best thing here is to return some other success code instead and have execCommand treat that as a case to return false.

Comment 16

[:Gijs (he/him)]

Comment on attachment 8962478 [details]
Bug 1446522 - don't return NS_ERROR_FAILURE for cut/copy/paste even when there's no selection,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/231308/diff/2-3/

Comment 17

[:Gijs (he/him)]

Comment on attachment 8962478 [details]
Bug 1446522 - don't return NS_ERROR_FAILURE for cut/copy/paste even when there's no selection,

Is this what you meant? :-)

Comment 18

[Neil Deakin]

Comment on attachment 8962478 [details]
Bug 1446522 - don't return NS_ERROR_FAILURE for cut/copy/paste even when there's no selection,

https://reviewboard.mozilla.org/r/231308/#review241446

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5b92973b85fc
don't return NS_ERROR_FAILURE for cut/copy/paste even when there's no selection, r=enndeakin+6102

Comment 20

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/5b92973b85fc