Closed Bug 1027560 Opened 6 years ago Closed 5 years ago

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

Categories

(Core :: DOM: Editor, defect)

33 Branch
x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: andyearnshaw, Assigned: Nika)

Details

(Keywords: dev-doc-complete)

Attachments

(1 file, 2 obsolete files)

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()
Component: General → Editor
Confirmed 33.0a1 (2014-07-14), win 7 x64
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → michael
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
Attachment #8605835 - Flags: review?(ehsan)
Attachment #8605835 - Flags: review?(ehsan) → review+
Please check in after Bug 1012662
Keywords: checkin-needed
sorry had to back this out for failing tests like https://treeherder.mozilla.org/logviewer.html#?job_id=10146347&repo=mozilla-inbound
Flags: needinfo?(michael)
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.
Attachment #8605835 - Attachment is obsolete: true
Attachment #8613682 - Flags: review?(ehsan)
Flags: needinfo?(michael)
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.)
Attachment #8613682 - Flags: review?(ehsan) → review?(james)
Interesting… those stub- files aren't really tests, but I didn't think the update script would delete the associated metadata.
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.
Attachment #8613682 - Attachment is obsolete: true
Attachment #8613682 - Flags: review?(james)
Attachment #8614116 - Flags: review?(ehsan)
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.
Attachment #8614116 - Flags: review?(ehsan) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e637319144d3
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Adding dev-doc-needed as the return value is not documented in the MDN reference page.
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.