Throw DOMException when invoked Document::execCommand on non-{X,}HTML documents

Throw DOMException when invoked Document::execCommand on non-{X,}HTML documents

Throw DOMException when invoked Document::execCommand on non-{X,}HTML documents as it may
lead to unexpected results. (e.g. InsertHTML execCommand running
HTML parser on top of XML document)

This change also aligns Blink better to other browsers and the
HTML Editing spec, which doesn't mention execCommand being applied
to non-HTML document:
https://dvcs.w3.org/hg/editing/raw-file/tip/editing.html

BUG=484974
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196652

[kouhei (in TOK), 2015-05-27 06:45:58]

kouhei@chromium.org changed reviewers:
+ esprehn@chromium.org, yosin@google.com

[kouhei (in TOK), 2015-05-27 06:46:10]

The CQ bit was checked by kouhei@chromium.org to run a CQ dry run

[kouhei (in TOK), 2015-05-27 06:46:15]

ptal

[commit-bot: I haz the power, 2015-05-27 06:46:27]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1155353002/20001

[yosin_UTC9, 2015-05-27 06:57:44]

yosin@chromium.org changed reviewers:
+ yosin@chromium.org

[yosin_UTC9, 2015-05-27 06:57:44]

lgtm
Thanks!

https://codereview.chromium.org/1155353002/diff/20001/LayoutTests/editing/exe...
File LayoutTests/editing/execCommand/non-html-document.html (right):

https://codereview.chromium.org/1155353002/diff/20001/LayoutTests/editing/exe...
LayoutTests/editing/execCommand/non-html-document.html:7:
shouldBeFalse("xmldoc.execCommand('bold, false, null')");
nit: you don't need to specify |false, null| for |document.execCommand()|.

[kouhei (in TOK), 2015-05-27 07:03:43]

The CQ bit was checked by kouhei@chromium.org

[kouhei (in TOK), 2015-05-27 07:03:43]

The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/1155353002/#ps40001
(title: "yosin review")

[commit-bot: I haz the power, 2015-05-27 07:03:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1155353002/40001

[kouhei (in TOK), 2015-05-27 07:04:20]

https://codereview.chromium.org/1155353002/diff/20001/LayoutTests/editing/exe...
File LayoutTests/editing/execCommand/non-html-document.html (right):

https://codereview.chromium.org/1155353002/diff/20001/LayoutTests/editing/exe...
LayoutTests/editing/execCommand/non-html-document.html:7:
shouldBeFalse("xmldoc.execCommand('bold, false, null')");
On 2015/05/27 06:57:44, Yosi_UTC9 wrote:
> nit: you don't need to specify |false, null| for |document.execCommand()|.

Done.

[sof, 2015-05-27 07:09:25]

sigbjornf@opera.com changed reviewers:
+ sigbjornf@opera.com

[sof, 2015-05-27 07:09:25]

Could you add some information about browser compatibility of this change to the
description?

(It aligns better.)

[kouhei (in TOK), 2015-05-27 07:11:36]

On 2015/05/27 07:09:25, sof wrote:
> Could you add some information about browser compatibility of this change to
the
> description?
> 
> (It aligns better.)

Good point. Done.

[esprehn, 2015-05-27 07:14:44]

On 2015/05/27 at 07:11:36, kouhei wrote:
> On 2015/05/27 07:09:25, sof wrote:
> > Could you add some information about browser compatibility of this change to
the
> > description?
> > 
> > (It aligns better.)
> 
> Good point. Done.

For what it's worth I tried in Firefox and I get:
xmldoc.execCommand is not a function

so it seems that we shouldn't even expose that method on the XMLDocument
interface...

[sof, 2015-05-27 07:16:07]

On 2015/05/27 07:14:44, esprehn wrote:
> On 2015/05/27 at 07:11:36, kouhei wrote:
> > On 2015/05/27 07:09:25, sof wrote:
> > > Could you add some information about browser compatibility of this change
to
> the
> > > description?
> > > 
> > > (It aligns better.)
> > 
> > Good point. Done.
> 
> For what it's worth I tried in Firefox and I get:
> xmldoc.execCommand is not a function
> 
> so it seems that we shouldn't even expose that method on the XMLDocument
> interface...

Trident and Presto throws.

[kouhei (in TOK), 2015-05-27 07:16:38]

The CQ bit was unchecked by kouhei@chromium.org

[kouhei (in TOK), 2015-05-27 07:23:57]

On 2015/05/27 07:16:07, sof wrote:
> On 2015/05/27 07:14:44, esprehn wrote:
> > On 2015/05/27 at 07:11:36, kouhei wrote:
> > > On 2015/05/27 07:09:25, sof wrote:
> > > > Could you add some information about browser compatibility of this
change
> to
> > the
> > > > description?
> > > > 
> > > > (It aligns better.)
> > > 
> > > Good point. Done.
> > 
> > For what it's worth I tried in Firefox and I get:
> > xmldoc.execCommand is not a function
> > 
> > so it seems that we shouldn't even expose that method on the XMLDocument
> > interface...
If I understand the spec correctly, Document == XMLDocument == HTMLDocument on
the latest spec:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

> 
> Trident and Presto throws.

The current CL returns false instead of throwing, should we also throw here to
match them?

[sof, 2015-05-27 08:53:45]

On 2015/05/27 07:23:57, kouhei wrote:
> On 2015/05/27 07:16:07, sof wrote:
> > On 2015/05/27 07:14:44, esprehn wrote:
> > > On 2015/05/27 at 07:11:36, kouhei wrote:
> > > > On 2015/05/27 07:09:25, sof wrote:
> > > > > Could you add some information about browser compatibility of this
> change
> > to
> > > the
> > > > > description?
> > > > > 
> > > > > (It aligns better.)
> > > > 
> > > > Good point. Done.
> > > 
> > > For what it's worth I tried in Firefox and I get:
> > > xmldoc.execCommand is not a function
> > > 
> > > so it seems that we shouldn't even expose that method on the XMLDocument
> > > interface...
> If I understand the spec correctly, Document == XMLDocument == HTMLDocument on
> the latest spec:
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> 
> > 
> > Trident and Presto throws.
> 
> The current CL returns false instead of throwing, should we also throw here to
> match them?

The DOMExceptions are ACCESS_DENIED and NOT_SUPPORTED_ERR, respectively.

Exceptions would flush out anyone depending on trunk's behavior.

[esprehn, 2015-06-03 04:04:55]

Did we fix this correctly by removing it from Document.idl and putting it in
HTMLDocument.idl yet?

[kouhei (in TOK), 2015-06-03 05:01:03]

On 2015/06/03 04:04:55, esprehn wrote:
> Did we fix this correctly by removing it from Document.idl and putting it in
> HTMLDocument.idl yet?

No, will do shortly

[kouhei (in TOK), 2015-06-05 09:29:12]

PTAL. Changed CL to throw NotSupportedError exception instead.

[kouhei (in TOK), 2015-06-05 15:03:39]

The CQ bit was checked by kouhei@chromium.org to run a CQ dry run

[kouhei (in TOK), 2015-06-05 15:03:40]

The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/1155353002/#ps60001
(title: "throw NotSupportedError")

[commit-bot: I haz the power, 2015-06-05 15:04:01]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1155353002/60001

[commit-bot: I haz the power, 2015-06-05 15:22:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-05 15:23:00]

Dry run: Try jobs failed on following builders:
  mac_blink_compile_dbg on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)

[kouhei (in TOK), 2015-06-08 05:01:50]

The CQ bit was checked by kouhei@chromium.org to run a CQ dry run

[kouhei (in TOK), 2015-06-08 05:01:51]

The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/1155353002/#ps80001
(title: "fix test / change to InvalidStateError")

[commit-bot: I haz the power, 2015-06-08 05:01:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1155353002/80001

[commit-bot: I haz the power, 2015-06-08 06:30:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-08 06:30:33]

Dry run: Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/6...)

[kouhei (in TOK), 2015-06-08 06:30:58]

The CQ bit was checked by kouhei@chromium.org to run a CQ dry run

[kouhei (in TOK), 2015-06-08 06:31:00]

The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/1155353002/#ps100001
(title: "revert accidental edit")

[commit-bot: I haz the power, 2015-06-08 06:31:09]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1155353002/100001

[kouhei (in TOK), 2015-06-08 06:31:19]

Yosi: Would you mind taking a look again?

[yosin_UTC9, 2015-06-08 07:10:40]

still lgtm
Thanks!

[commit-bot: I haz the power, 2015-06-08 07:27:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-08 07:27:02]

Dry run: This issue passed the CQ dry run.

[kouhei (in TOK), 2015-06-08 07:27:24]

The CQ bit was checked by kouhei@chromium.org

[commit-bot: I haz the power, 2015-06-08 07:27:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1155353002/100001

[commit-bot: I haz the power, 2015-06-08 07:33:23]

Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196652