1027560 - document.execCommand() incorrectly throws NS_ERROR_FAILURE for non-editables

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Andy Earnshaw]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/35.0.1916.114 Safari/537.36

Steps to reproduce:

1. Create a HTML document, can be empty.
2. Execute the following JavaScript:

    document.execCommand('inserttext', false, 'foo');


Actual results:

The console logs the an unhelpful "NS_ERROR_FAILURE" exception, with no reasonable error message to explain the problem.


Expected results:

No error should be thrown, the result of `document.execCommand()` should be false.  The HTML Editing API spec defines the outlines the following steps[1]:

> When the execCommand(command, show UI, value) method on the HTMLDocument 
> interface is invoked, the user agent must run the following steps:

>  1. If only one argument was provided, let show UI be false.
>  2. If only one or two arguments were provided, let value be the empty string.
>  3. If command is not supported or not enabled, return false.

Chrome and IE both exhibit the correct behaviour, with both even allowing some commands to be used to edit the text in <input> and <textarea> elements.

[1]: https://dvcs.w3.org/hg/editing/raw-file/tip/editing.html#execcommand()

Comment 1

[Paul Silaghi, QA [:pauly]]

Confirmed 33.0a1 (2014-07-14), win 7 x64

Comment 2

[Nika Layzell [:nika] (ni? for response)]

Attachment: Return false from document.execCommand() and friends when not in editable document

Patch is based off bug 1012662 - will probably merge more cleanly if that bug has already been merged.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec92cce1ffcb

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Please check in after Bug 1012662

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fdc2aa9e612f

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fdc2aa9e612f

Comment 6

[Carsten Book [:Tomcat]]

sorry had to back this out for failing tests like https://treeherder.mozilla.org/logviewer.html#?job_id=10146347&repo=mozilla-inbound

Comment 7

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3a55846ffd7

Comment 8

[Nika Layzell [:nika] (ni? for response)]

Attachment: Return false from document.execCommand() and friends when not in editable document

Updated patch which fixes the test bustage which I missed in the previous try push.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bec802fef2f8

I'm not sure about some of the file deletions that the updater tool did, so I'm asking for review again.

Comment 9

[(no longer active)]

Comment on attachment 8613682 [details] [diff] [review]
Return false from document.execCommand() and friends when not in editable document

Review of attachment 8613682 [details] [diff] [review]:
-----------------------------------------------------------------

Whoa!  Those deletions are definitely wrong.  James, what is going on here?

(Michael, you can probably revert them manually for now.  Nothing in testing/web-platform/meta/service-workers/ should change in this patch.)

Comment 10

[James Graham [:jgraham]]

Interesting… those stub- files aren't really tests, but I didn't think the update script would delete the associated metadata.

Comment 11

[Nika Layzell [:nika] (ni? for response)]

Attachment: Return false from document.execCommand() and friends when not in editable document

New version which doesn't delete a ton of random files - I can run try again on it if it makes sense to do so.

Comment 12

[(no longer active)]

Comment on attachment 8614116 [details] [diff] [review]
Return false from document.execCommand() and friends when not in editable document

Review of attachment 8614116 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!  I don't think this needs another try run.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e637319144d3

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e637319144d3

Comment 15

[Jean-Yves Perrier [:teoli]]

Adding dev-doc-needed as the return value is not documented in the MDN reference page.

Comment 16

[Jean-Yves Perrier [:teoli]]

Updated
https://developer.mozilla.org/en-US/docs/Web/API/Document/execCommand
and
https://developer.mozilla.org/en-US/Firefox/Releases/41#HTML_Editing_API