[Editing][DOM][CodeHealth] Make Node::isContentEditable and Node::isRichEditable global functions.

[Editing][DOM][CodeHealth] Make Node::isContentEditable and Node::isRichEditable
global functions.

Make Node::isContentEditable and Node::isRichEditable
global functions.
This CL also re-introduce HTMLElement::isContentEditable because
 it is a web-exposed IDL attribute.

This is a preparation to move these functions from core/dom to core/editing/

TEST=No change in behavior

Committed: https://crrev.com/4b266be31a4596cf8b76bdce903c4b138803dbad
Cr-Commit-Position: refs/heads/master@{#407100}

[yoichio, 2016-07-07 09:13:04]

Description was changed from

==========
node edit

BUG=
==========

to

==========
Make Node.*editable* functions static

This is preparation to move these function from Node to core/editing/

TEST=No change in behavior
==========

[yoichio, 2016-07-07 09:13:04]

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

[yoichio, 2016-07-07 09:14:17]

PTAL.
but I want to wait shipping another patch conflicting:
https://codereview.chromium.org/2120913002/

[yosin_UTC9, 2016-07-07 09:34:54]

Since hasEditableStyle() and isEditableToAccessibility() don't take nullptr,
let's use |const Node&|.

https://codereview.chromium.org/2121313003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2121313003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Node.cpp:550: bool
Node::hasEditableStyle(const Node* node, EditableLevel editableLevel,
UserSelectAllTreatment treatment)
s/const Node*/const Node&/

https://codereview.chromium.org/2121313003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Node.cpp:582: bool
Node::isEditableToAccessibility(const Node* node, EditableLevel editableLevel)
s/const Node*/const Node&/

[yoichio, 2016-07-21 06:47:14]

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

[commit-bot: I haz the power, 2016-07-21 06:47:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yoichio, 2016-07-21 06:47:45]

Description was changed from

==========
Make Node.*editable* functions static

This is preparation to move these function from Node to core/editing/

TEST=No change in behavior
==========

to

==========
[Editing][DOM][CodeHealth] Make Node::isContentEditable static

Make Node::isContentEditable and ::isContentRichEditable static.

This is preparation to move these function from Node to core/editing/

TEST=No change in behavior
==========

[yoichio, 2016-07-21 06:47:45]

yoichio@chromium.org changed reviewers:
+ tkent@chromium.org

[yoichio, 2016-07-21 08:14:03]

PTAL

[yosin_UTC9, 2016-07-21 08:25:02]

https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLElement.cpp (right):

https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLElement.cpp:589: bool
HTMLElement::isContentEditable() const
Why do we need to have |HTMLElement::isContentEditable()| even if we get rid of
|Node::isContentEditable()|?
Will we get rid of this?

[tkent, 2016-07-21 08:28:44]

lgtm

https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebNode.cpp (right):

https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebNode.cpp:172: bool WebNode::isContentEditable()
const
I found this function was not used and we should remove it though it's unrelated
to this CL.

[yosin_UTC9, 2016-07-21 08:28:57]

Please update description:

>Make Node::isContentEditable and ::isContentRichEditable static.
Make Node::isContentEditable and Node::isRichEditable global functions.

>This is preparation to move these function from Node to core/editing/
s/preparation/a preparation/
s/these function/these functions/
s|Node|core/dom/|

[tkent, 2016-07-21 08:29:55]

https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLElement.cpp (right):

https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLElement.cpp:589: bool
HTMLElement::isContentEditable() const
On 2016/07/21 at 08:25:02, Yosi_UTC9 wrote:
> Why do we need to have |HTMLElement::isContentEditable()| even if we get rid
of |Node::isContentEditable()|?
> Will we get rid of this?

HTMLElement::isContentEditable is a web-exposed IDL attribute.

[yoichio, 2016-07-21 08:35:40]

Description was changed from

==========
[Editing][DOM][CodeHealth] Make Node::isContentEditable static

Make Node::isContentEditable and ::isContentRichEditable static.

This is preparation to move these function from Node to core/editing/

TEST=No change in behavior
==========

to

==========
[Editing][DOM][CodeHealth] Make Node::isContentEditable and Node::isRichEditable

global functions.

Make Node::isContentEditable and ::isContentRichEditable static.
This CL also re-introduce HTMLElement::isContentEditable because
 it is a web-exposed IDL attribute.

This is a preparation to move these functions from core/dom to core/editing/

TEST=No change in behavior
==========

[yoichio, 2016-07-21 08:36:11]

Description was changed from

==========
[Editing][DOM][CodeHealth] Make Node::isContentEditable and Node::isRichEditable

global functions.

Make Node::isContentEditable and ::isContentRichEditable static.
This CL also re-introduce HTMLElement::isContentEditable because
 it is a web-exposed IDL attribute.

This is a preparation to move these functions from core/dom to core/editing/

TEST=No change in behavior
==========

to

==========
[Editing][DOM][CodeHealth] Make Node::isContentEditable and Node::isRichEditable

global functions.

Make Node::isContentEditable and Node::isRichEditable 
global functions.
This CL also re-introduce HTMLElement::isContentEditable because
 it is a web-exposed IDL attribute.

This is a preparation to move these functions from core/dom to core/editing/

TEST=No change in behavior
==========

[yoichio, 2016-07-21 08:36:33]

Update description.

https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebNode.cpp (right):

https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebNode.cpp:172: bool WebNode::isContentEditable()
const
On 2016/07/21 08:28:44, tkent wrote:
> I found this function was not used and we should remove it though it's
unrelated
> to this CL.

Acknowledged.

[yosin_UTC9, 2016-07-21 08:44:30]

On 2016/07/21 at 08:29:55, tkent wrote:
>
https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/html/HTMLElement.cpp (right):
> 
>
https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/html/HTMLElement.cpp:589: bool
HTMLElement::isContentEditable() const
> On 2016/07/21 at 08:25:02, Yosi_UTC9 wrote:
> > Why do we need to have |HTMLElement::isContentEditable()| even if we get rid
of |Node::isContentEditable()|?
> > Will we get rid of this?
> 
> HTMLElement::isContentEditable is a web-exposed IDL attribute.

Thanks for explanation.

Can we rename |HTMLElement::isContentEditable()| to
|HTMLElement::isContentEditableForBindings| or something?
To make C++ code use global function |isContentEditable()|?

[commit-bot: I haz the power, 2016-07-21 10:39:15]

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

[commit-bot: I haz the power, 2016-07-21 10:39:17]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[yoichio, 2016-07-22 04:20:48]

On 2016/07/21 08:44:30, Yosi_UTC9 wrote:
> On 2016/07/21 at 08:29:55, tkent wrote:
> >
>
https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/html/HTMLElement.cpp (right):
> > 
> >
>
https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/html/HTMLElement.cpp:589: bool
> HTMLElement::isContentEditable() const
> > On 2016/07/21 at 08:25:02, Yosi_UTC9 wrote:
> > > Why do we need to have |HTMLElement::isContentEditable()| even if we get
rid
> of |Node::isContentEditable()|?
> > > Will we get rid of this?
> > 
> > HTMLElement::isContentEditable is a web-exposed IDL attribute.
> 
> Thanks for explanation.
> 
> Can we rename |HTMLElement::isContentEditable()| to
> |HTMLElement::isContentEditableForBindings| or something?
> To make C++ code use global function |isContentEditable()|?

What do you mean?

[yosin_UTC9, 2016-07-22 04:46:12]

On 2016/07/22 at 04:20:48, yoichio wrote:
> On 2016/07/21 08:44:30, Yosi_UTC9 wrote:
> > On 2016/07/21 at 08:29:55, tkent wrote:
> > >
> >
https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
> > > File third_party/WebKit/Source/core/html/HTMLElement.cpp (right):
> > > 
> > >
> >
https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
> > > third_party/WebKit/Source/core/html/HTMLElement.cpp:589: bool
> > HTMLElement::isContentEditable() const
> > > On 2016/07/21 at 08:25:02, Yosi_UTC9 wrote:
> > > > Why do we need to have |HTMLElement::isContentEditable()| even if we get
rid
> > of |Node::isContentEditable()|?
> > > > Will we get rid of this?
> > > 
> > > HTMLElement::isContentEditable is a web-exposed IDL attribute.
> > 
> > Thanks for explanation.
> > 
> > Can we rename |HTMLElement::isContentEditable()| to
> > |HTMLElement::isContentEditableForBindings| or something?
> > To make C++ code use global function |isContentEditable()|?
> 
> What do you mean?

When we have |HTMLElement::isContentEditable()|, we can write

HTMLElement* element = ...;

element.isContentEditable()
isContentEditable(*element)

I would like to use only |isContentEditable(*element)| to simplify code.

[yoichio, 2016-07-22 05:10:20]

On 2016/07/22 04:46:12, Yosi_UTC9 wrote:
> On 2016/07/22 at 04:20:48, yoichio wrote:
> > On 2016/07/21 08:44:30, Yosi_UTC9 wrote:
> > > On 2016/07/21 at 08:29:55, tkent wrote:
> > > >
> > >
>
https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
> > > > File third_party/WebKit/Source/core/html/HTMLElement.cpp (right):
> > > > 
> > > >
> > >
>
https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
> > > > third_party/WebKit/Source/core/html/HTMLElement.cpp:589: bool
> > > HTMLElement::isContentEditable() const
> > > > On 2016/07/21 at 08:25:02, Yosi_UTC9 wrote:
> > > > > Why do we need to have |HTMLElement::isContentEditable()| even if we
get
> rid
> > > of |Node::isContentEditable()|?
> > > > > Will we get rid of this?
> > > > 
> > > > HTMLElement::isContentEditable is a web-exposed IDL attribute.
> > > 
> > > Thanks for explanation.
> > > 
> > > Can we rename |HTMLElement::isContentEditable()| to
> > > |HTMLElement::isContentEditableForBindings| or something?
> > > To make C++ code use global function |isContentEditable()|?
> > 
> > What do you mean?
> 
> When we have |HTMLElement::isContentEditable()|, we can write
> 
> HTMLElement* element = ...;
> 
> element.isContentEditable()
> isContentEditable(*element)
> 
> I would like to use only |isContentEditable(*element)| to simplify code.

I see. Will do later.

[yoichio, 2016-07-22 05:10:24]

The CQ bit was checked by yoichio@chromium.org

[yoichio, 2016-07-22 05:10:25]

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

[commit-bot: I haz the power, 2016-07-22 05:10:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-22 08:19:40]

Description was changed from

==========
[Editing][DOM][CodeHealth] Make Node::isContentEditable and Node::isRichEditable

global functions.

Make Node::isContentEditable and Node::isRichEditable 
global functions.
This CL also re-introduce HTMLElement::isContentEditable because
 it is a web-exposed IDL attribute.

This is a preparation to move these functions from core/dom to core/editing/

TEST=No change in behavior
==========

to

==========
[Editing][DOM][CodeHealth] Make Node::isContentEditable and Node::isRichEditable

global functions.

Make Node::isContentEditable and Node::isRichEditable 
global functions.
This CL also re-introduce HTMLElement::isContentEditable because
 it is a web-exposed IDL attribute.

This is a preparation to move these functions from core/dom to core/editing/

TEST=No change in behavior
==========

[commit-bot: I haz the power, 2016-07-22 08:19:41]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-07-22 08:21:02]

Description was changed from

==========
[Editing][DOM][CodeHealth] Make Node::isContentEditable and Node::isRichEditable

global functions.

Make Node::isContentEditable and Node::isRichEditable 
global functions.
This CL also re-introduce HTMLElement::isContentEditable because
 it is a web-exposed IDL attribute.

This is a preparation to move these functions from core/dom to core/editing/

TEST=No change in behavior
==========

to

==========
[Editing][DOM][CodeHealth] Make Node::isContentEditable and Node::isRichEditable
global functions.

Make Node::isContentEditable and Node::isRichEditable
global functions.
This CL also re-introduce HTMLElement::isContentEditable because
 it is a web-exposed IDL attribute.

This is a preparation to move these functions from core/dom to core/editing/

TEST=No change in behavior

Committed: https://crrev.com/4b266be31a4596cf8b76bdce903c4b138803dbad
Cr-Commit-Position: refs/heads/master@{#407100}
==========

[commit-bot: I haz the power, 2016-07-22 08:21:03]

Patchset 3 (id:??) landed as
https://crrev.com/4b266be31a4596cf8b76bdce903c4b138803dbad
Cr-Commit-Position: refs/heads/master@{#407100}

[tkent, 2016-07-25 01:54:16]

https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebNode.cpp (right):

https://codereview.chromium.org/2121313003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebNode.cpp:172: bool WebNode::isContentEditable()
const
On 2016/07/21 at 08:36:33, yoichio wrote:
> On 2016/07/21 08:28:44, tkent wrote:
> > I found this function was not used and we should remove it though it's
unrelated
> > to this CL.
> 
> Acknowledged.

I was wrong.  It's used only for Android. 
RenderWidget::showUnhandledTapUIIfNeeded().