Apply vertical-align style of <sub> and <sup> to child elements.

Apply vertical-align style of <sub> and <sup> to child elements.

If the selected element has <sub> or <sup> ancestor element,
apply the corresponding style(vertical-align) to it so that
document.queryCommandState() works fine on the selected element.

<sub> and <sup> tags are represented with CSS vertical-align
property but they are not inherited. Therefore, we need to apply the
property to child elements because <sub> and <sup> tag can have
child elements such as <i> or <b> tag.

BUG=582225
TEST=editing/execCommand/queryCommandState-04.html
     editing/execCommand/queryCommandState-04-mac.html

Committed: https://crrev.com/dfe3d34820670521568ca7dfe681f701da94325b
Cr-Commit-Position: refs/heads/master@{#396763}

[joone, 2016-05-17 00:26:23]

joone.hur@intel.com changed reviewers:
+ yosin@chromium.org

[joone, 2016-05-17 00:26:23]

Hi, yosin@ could you review this CL?

[yosin_UTC9, 2016-05-17 01:15:48]

The approach seems good. Nits for coding style.

https://codereview.chromium.org/1986563002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/EditingStyle.cpp (right):

https://codereview.chromium.org/1986563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingStyle.cpp:396: if (propertyID ==
CSSPropertyVerticalAlign) {
nit: Can we simplify this?

m_isVerticalAlign = propertyID == CSSProeprtyVerticalAlign && (value == "sub" ||
value == "super");

https://codereview.chromium.org/1986563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingStyle.cpp:753: for (Node* ancestor
= node; ancestor; ancestor = ancestor->parentNode()) {
nit: Please use range-for

for (Node* runner : NodeTraversal::inclusiveAncestorsOf(*node)) {
 ...
}

Since, NodeTraversal::inclusiveAncestorsOf(*node) == node and ancestorsOf(node),

variable |ancestor| isn't match.

https://codereview.chromium.org/1986563002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/EditingStyle.h (right):

https://codereview.chromium.org/1986563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingStyle.h:166: bool
m_isVerticalAlign;
nit: Let's use inline initializer to avoid initializing |m_isVerticalAlign| in
each constructor.
bool m_isVerticalAlign = false;

Note: We may want to have using delegating constructor and using inline
initializer in other patches; one for delegating constructor and another for
inline initializer.

[joone, 2016-05-17 23:01:33]

https://codereview.chromium.org/1986563002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/EditingStyle.cpp (right):

https://codereview.chromium.org/1986563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingStyle.cpp:396: if (propertyID ==
CSSPropertyVerticalAlign) {
On 2016/05/17 01:15:48, Yosi_UTC9 wrote:
> nit: Can we simplify this?
> 
> m_isVerticalAlign = propertyID == CSSProeprtyVerticalAlign && (value == "sub"
||
> value == "super");

Done.

https://codereview.chromium.org/1986563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingStyle.cpp:753: for (Node* ancestor
= node; ancestor; ancestor = ancestor->parentNode()) {
On 2016/05/17 01:15:48, Yosi_UTC9 wrote:
> nit: Please use range-for
> 
> for (Node* runner : NodeTraversal::inclusiveAncestorsOf(*node)) {
>  ...
> }
> 
> Since, NodeTraversal::inclusiveAncestorsOf(*node) == node and
ancestorsOf(node),
> 
> variable |ancestor| isn't match.
> 
> 
> 

Done.

https://codereview.chromium.org/1986563002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/EditingStyle.h (right):

https://codereview.chromium.org/1986563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/EditingStyle.h:166: bool
m_isVerticalAlign;
On 2016/05/17 01:15:48, Yosi_UTC9 wrote:
> nit: Let's use inline initializer to avoid initializing |m_isVerticalAlign| in
> each constructor.
> bool m_isVerticalAlign = false;
> 
> Note: We may want to have using delegating constructor and using inline
> initializer in other patches; one for delegating constructor and another for
> inline initializer.

Ok, I will prepare for the patches.

[joone, 2016-05-18 01:10:38]

Can I create a separated CL for Mac?

Mac is different from other platforms when getting a style from selection.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[yosin_UTC9, 2016-05-18 02:20:00]

On 2016/05/18 at 01:10:38, joone.hur wrote:
> Can I create a separated CL for Mac?
> 
> Mac is different from other platforms when getting a style from selection.
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
Do you mean this CL doesn't cover Mac case?
Could you tell me differences of Mac than other platforms?
Sorry, I couldn't identify it from an implementation of |stateStyle()|.

[joone, 2016-05-18 05:30:20]

On 2016/05/18 02:20:00, Yosi_UTC9 wrote:
> On 2016/05/18 at 01:10:38, joone.hur wrote:
> > Can I create a separated CL for Mac?
> > 
> > Mac is different from other platforms when getting a style from selection.
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> Do you mean this CL doesn't cover Mac case?
> Could you tell me differences of Mac than other platforms?
> Sorry, I couldn't identify it from an implementation of |stateStyle()|.

Yes, this CL doesn't cover Mac case. I came to know through checking the test
failure on Mac.
The following method is called when we run document.queryCommandState().

static TriState stateStyle(LocalFrame& frame, CSSPropertyID propertyID, const
char* desiredValue)
{
    // for Mac
    if (frame.editor().behavior().shouldToggleStyleBasedOnStartOfSelection())
        return frame.editor().selectionStartHasStyle(propertyID, desiredValue) ?
TrueTriState : FalseTriState;
   // for Linux and Windows 
   return frame.editor().selectionHasStyle(propertyID, desiredValue);
}

However, the way to get the styles from the selection is different in Mac. See
the comment below:
    // On Mac, style is considered present when present at the beginning of
selection. On other platforms,
    // style has to be present throughout the selection.
    bool shouldToggleStyleBasedOnStartOfSelection() const { return m_type ==
EditingMacBehavior; }

[yosin_UTC9, 2016-05-18 07:09:22]

On 2016/05/18 at 05:30:20, joone.hur wrote:
> On 2016/05/18 02:20:00, Yosi_UTC9 wrote:
> > On 2016/05/18 at 01:10:38, joone.hur wrote:
> > > Can I create a separated CL for Mac?
> > > 
> > > Mac is different from other platforms when getting a style from selection.
> > >
> >
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> > Do you mean this CL doesn't cover Mac case?
> > Could you tell me differences of Mac than other platforms?
> > Sorry, I couldn't identify it from an implementation of |stateStyle()|.
> 
> Yes, this CL doesn't cover Mac case. I came to know through checking the test
failure on Mac.
> The following method is called when we run document.queryCommandState().
> 
> static TriState stateStyle(LocalFrame& frame, CSSPropertyID propertyID, const
char* desiredValue)
> {
>     // for Mac
>     if (frame.editor().behavior().shouldToggleStyleBasedOnStartOfSelection())
>         return frame.editor().selectionStartHasStyle(propertyID, desiredValue)
? TrueTriState : FalseTriState;
>    // for Linux and Windows 
>    return frame.editor().selectionHasStyle(propertyID, desiredValue);
> }
> 
> However, the way to get the styles from the selection is different in Mac. See
the comment below:
>     // On Mac, style is considered present when present at the beginning of
selection. On other platforms,
>     // style has to be present throughout the selection.
>     bool shouldToggleStyleBasedOnStartOfSelection() const { return m_type ==
EditingMacBehavior; }

Thanks for explanation.

Since, document.queryCommandState() is available for all platform. It is better
to this patch covers all platform.
We have internals.settings.setEditingBehavior(platformName,); // platformName =
"win", "mac", etc.
So, you can write a single test file to cover all platforms.

[joone, 2016-05-19 22:58:45]

Patchset #5 (id:80001) has been deleted

[joone, 2016-05-19 23:03:23]

Patchset #5 (id:100001) has been deleted

[joone, 2016-05-20 00:54:55]

On 2016/05/18 07:09:22, Yosi_UTC9 wrote:
> 
> Since, document.queryCommandState() is available for all platform. It is
better
> to this patch covers all platform.
> We have internals.settings.setEditingBehavior(platformName,); // platformName
=
> "win", "mac", etc.
> So, you can write a single test file to cover all platforms.

yosin@ This api was helpful, which allows me to test Mac editing behavior on
Linux.
I've updated the CL that supports Mac editing behavior. Thanks!

[joone, 2016-05-25 06:43:55]

Description was changed from

==========
Apply vertical-align style of <sub> and <sup> to child elements.

If the selected element has <sub> or <sup> ancestor element, apply the
corrsponding
style(vertical-align) to it so that document.queryCommandState() works with the
sytle.

BUG=582225
TEST=editing/execCommand/queryCommandState-04.html
==========

to

==========
Apply vertical-align style of <sub> and <sup> to child elements.

If the selected element has <sub> or <sup> ancestor element,
apply the corresponding style(vertical-align) to it so that
document.queryCommandState() works fine on the selected element. 

<sub> and <sup> tags are represented with CSS vertical-align
property but they are not inherited. Therefore, we need to apply the property to
child elements because <sub> and <sup> can have child elements such as <i> tag
or <b> tag.

BUG=582225
TEST=editing/execCommand/queryCommandState-04.html
==========

[joone, 2016-05-25 06:44:21]

Description was changed from

==========
Apply vertical-align style of <sub> and <sup> to child elements.

If the selected element has <sub> or <sup> ancestor element,
apply the corresponding style(vertical-align) to it so that
document.queryCommandState() works fine on the selected element. 

<sub> and <sup> tags are represented with CSS vertical-align
property but they are not inherited. Therefore, we need to apply the property to
child elements because <sub> and <sup> can have child elements such as <i> tag
or <b> tag.

BUG=582225
TEST=editing/execCommand/queryCommandState-04.html
==========

to

==========
Apply vertical-align style of <sub> and <sup> to child elements.

If the selected element has <sub> or <sup> ancestor element,
apply the corresponding style(vertical-align) to it so that
document.queryCommandState() works fine on the selected element. 

<sub> and <sup> tags are represented with CSS vertical-align
property but they are not inherited. Therefore, we need to apply the 
property to child elements because <sub> and <sup> can have child 
elements such as <i> tag or <b> tag.

BUG=582225
TEST=editing/execCommand/queryCommandState-04.html
==========

[joone, 2016-05-25 06:45:26]

Description was changed from

==========
Apply vertical-align style of <sub> and <sup> to child elements.

If the selected element has <sub> or <sup> ancestor element,
apply the corresponding style(vertical-align) to it so that
document.queryCommandState() works fine on the selected element. 

<sub> and <sup> tags are represented with CSS vertical-align
property but they are not inherited. Therefore, we need to apply the 
property to child elements because <sub> and <sup> can have child 
elements such as <i> tag or <b> tag.

BUG=582225
TEST=editing/execCommand/queryCommandState-04.html
==========

to

==========
Apply vertical-align style of <sub> and <sup> to child elements.

If the selected element has <sub> or <sup> ancestor element,
apply the corresponding style(vertical-align) to it so that
document.queryCommandState() works fine on the selected element. 

<sub> and <sup> tags are represented with CSS vertical-align
property but they are not inherited. Therefore, we need to apply the 
property to child elements because <sub> and <sup> tag can have
child elements such as <i> or <b> tag.

BUG=582225
TEST=editing/execCommand/queryCommandState-04.html
==========

[yosin_UTC9, 2016-05-25 07:01:20]

On 2016/05/20 at 00:54:55, joone.hur wrote:
> On 2016/05/18 07:09:22, Yosi_UTC9 wrote:
> > 
> > Since, document.queryCommandState() is available for all platform. It is
better
> > to this patch covers all platform.
> > We have internals.settings.setEditingBehavior(platformName,); //
platformName =
> > "win", "mac", etc.
> > So, you can write a single test file to cover all platforms.
> 
> yosin@ This api was helpful, which allows me to test Mac editing behavior on
Linux.
> I've updated the CL that supports Mac editing behavior. Thanks!

Could you add tests for Mac?

[joone, 2016-05-25 07:28:34]

On 2016/05/25 07:01:20, Yosi_UTC9 wrote:
> On 2016/05/20 at 00:54:55, joone.hur wrote:
> > On 2016/05/18 07:09:22, Yosi_UTC9 wrote:
> > > 
> > > Since, document.queryCommandState() is available for all platform. It is
> better
> > > to this patch covers all platform.
> > > We have internals.settings.setEditingBehavior(platformName,); //
> platformName =
> > > "win", "mac", etc.
> > > So, you can write a single test file to cover all platforms.
> > 
> > yosin@ This api was helpful, which allows me to test Mac editing behavior on
> Linux.
> > I've updated the CL that supports Mac editing behavior. Thanks!
> 
> Could you add tests for Mac?

The test case I added can cover Mac. It only failed on Mac, which was fixed with
the patch set 5.

[yosin_UTC9, 2016-05-25 08:08:39]

On 2016/05/25 at 07:28:34, joone.hur wrote:
> On 2016/05/25 07:01:20, Yosi_UTC9 wrote:
> > On 2016/05/20 at 00:54:55, joone.hur wrote:
> > > On 2016/05/18 07:09:22, Yosi_UTC9 wrote:
> > > > 
> > > > Since, document.queryCommandState() is available for all platform. It is
> > better
> > > > to this patch covers all platform.
> > > > We have internals.settings.setEditingBehavior(platformName,); //
> > platformName =
> > > > "win", "mac", etc.
> > > > So, you can write a single test file to cover all platforms.
> > > 
> > > yosin@ This api was helpful, which allows me to test Mac editing behavior
on
> > Linux.
> > > I've updated the CL that supports Mac editing behavior. Thanks!
> > 
> > Could you add tests for Mac?
> 
> The test case I added can cover Mac. It only failed on Mac, which was fixed
with the patch set 5.

I mean the test file test utilizing setEditingBehavior() to test both "win" and
"mac" on any platform.
For example,

function test1(platform) {
  if (window.internals) {
    internals.setEditingBehavior(platform);
  ....
}

function test2(platform) {
  if (window.internals)
    internals.setEditingBehavior(platform);
  ....
}

var isMac = navigator.platform.indexOf('Mac') == 0;

if (window.internals) {
   test(test1("win"));
   test(test2("win"));
   test(test1("mac"));
   test(test2("mac"));
if (isMac) {
   test(test1("mac"));
   test(test2("mac"));
} else {
   test(test1("win"));
   test(test2("win"));
}

Or better control flow. ;-)

[joone, 2016-05-27 08:02:26]

Patchset #7 (id:160001) has been deleted

[joone, 2016-05-27 08:04:36]

Description was changed from

==========
Apply vertical-align style of <sub> and <sup> to child elements.

If the selected element has <sub> or <sup> ancestor element,
apply the corresponding style(vertical-align) to it so that
document.queryCommandState() works fine on the selected element. 

<sub> and <sup> tags are represented with CSS vertical-align
property but they are not inherited. Therefore, we need to apply the 
property to child elements because <sub> and <sup> tag can have
child elements such as <i> or <b> tag.

BUG=582225
TEST=editing/execCommand/queryCommandState-04.html
==========

to

==========
Apply vertical-align style of <sub> and <sup> to child elements.

If the selected element has <sub> or <sup> ancestor element,
apply the corresponding style(vertical-align) to it so that
document.queryCommandState() works fine on the selected element. 

<sub> and <sup> tags are represented with CSS vertical-align
property but they are not inherited. Therefore, we need to apply the 
property to child elements because <sub> and <sup> tag can have
child elements such as <i> or <b> tag.

BUG=582225
TEST=editing/execCommand/queryCommandState-04.html
     editing/execCommand/queryCommandState-04-mac.html
==========

[joone, 2016-05-27 08:11:40]

On 2016/05/25 08:08:39, Yosi_UTC9 wrote:
> On 2016/05/25 at 07:28:34, joone.hur wrote:
> > On 2016/05/25 07:01:20, Yosi_UTC9 wrote:
> > > On 2016/05/20 at 00:54:55, joone.hur wrote:
> > > > On 2016/05/18 07:09:22, Yosi_UTC9 wrote:
> > > > > 
> > > > > Since, document.queryCommandState() is available for all platform. It
is
> > > better
> > > > > to this patch covers all platform.
> > > > > We have internals.settings.setEditingBehavior(platformName,); //
> > > platformName =
> > > > > "win", "mac", etc.
> > > > > So, you can write a single test file to cover all platforms.
> > > > 
> > > > yosin@ This api was helpful, which allows me to test Mac editing
behavior
> on
> > > Linux.
> > > > I've updated the CL that supports Mac editing behavior. Thanks!
> > > 
> > > Could you add tests for Mac?
> > 
> > The test case I added can cover Mac. It only failed on Mac, which was fixed
> with the patch set 5.
> 
> I mean the test file test utilizing setEditingBehavior() to test both "win"
and
> "mac" on any platform.
> For example,
> 
> function test1(platform) {
>   if (window.internals) {
>     internals.setEditingBehavior(platform);
>   ....
> }
> 
> function test2(platform) {
>   if (window.internals)
>     internals.setEditingBehavior(platform);
>   ....
> }
> 
> var isMac = navigator.platform.indexOf('Mac') == 0;
> 
> if (window.internals) {
>    test(test1("win"));
>    test(test2("win"));
>    test(test1("mac"));
>    test(test2("mac"));
> if (isMac) {
>    test(test1("mac"));
>    test(test2("mac"));
> } else {
>    test(test1("win"));
>    test(test2("win"));
> }
> 
> Or better control flow. ;-)

Thanks for the example. I've added a test file for Mac.

[yosin_UTC9, 2016-05-30 01:28:12]

C++ changes are OK.
We need to simplify test file.

https://codereview.chromium.org/1986563002/diff/180001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-04-mac.html
(right):

https://codereview.chromium.org/1986563002/diff/180001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-04-mac.html:1:
<!doctype HTML>
Could you integrate this test into CommandState-04.html?

There are two cases whether window.internals available or not:

No window.internals: run sub and super test
With window.internals: run sub and super test for "win" and "mac".

e.g.

for (const platform of ['mac', 'win']) {
  if (window.internals)
    internals.settings.setEditingBehavior(platform);
  test(function() { ... sub test ... }, `${platform}: run
queryCommandState('subscript')....`);
  test(function() { ... super test ... }, `${platform}: run
queryCommandState('superscript') ...`);
  if (!window.internals)
    break;
}

[joone, 2016-05-30 06:07:39]

Patchset #8 (id:200001) has been deleted

[joone, 2016-05-30 06:20:49]

On 2016/05/30 01:28:12, Yosi_UTC9 wrote:
> C++ changes are OK.
> We need to simplify test file.
> 
>
https://codereview.chromium.org/1986563002/diff/180001/third_party/WebKit/Lay...
> File
>
third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-04-mac.html
> (right):
> 
>
https://codereview.chromium.org/1986563002/diff/180001/third_party/WebKit/Lay...
>
third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-04-mac.html:1:
> <!doctype HTML>
> Could you integrate this test into CommandState-04.html?
> 
> There are two cases whether window.internals available or not:
> 
> No window.internals: run sub and super test
> With window.internals: run sub and super test for "win" and "mac".
> 
> e.g.
> 
> for (const platform of ['mac', 'win']) {
>   if (window.internals)
>     internals.settings.setEditingBehavior(platform);
>   test(function() { ... sub test ... }, `${platform}: run
> queryCommandState('subscript')....`);
>   test(function() { ... super test ... }, `${platform}: run
> queryCommandState('superscript') ...`);
>   if (!window.internals)
>     break;
> }

Hi yosin@,

I've merged two test files into one and made the for-loop break a bit earlier.
It can run the test function six times for Blink layout test, but it can only
run it two times when we open the test file with Chrome browser because
window.internal is not supported. Thanks for the comment.

[joone, 2016-05-30 06:24:45]

Patchset #8 (id:220001) has been deleted

[yosin_UTC9, 2016-05-30 06:43:42]

https://codereview.chromium.org/1986563002/diff/240001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-04.html
(right):

https://codereview.chromium.org/1986563002/diff/240001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-04.html:33:
if (window.internals)
nit: How about this? Since, we don't want to write descriptions twice.
function test_all(platform) {
  if (platform)
    window.internals.setEditingBehavior(platform);
  test(test_sub, `${platform} run queryCommandState('subscript')`);
  test(test_sup, `${platform} run queryCommandState('superscript')`);
}

(windows.internal ? [''] : ['mac', 'win]).forEach(platform, test_all);

[joone, 2016-05-30 07:30:30]

On 2016/05/30 06:43:42, Yosi_UTC9 wrote:
>
https://codereview.chromium.org/1986563002/diff/240001/third_party/WebKit/Lay...
> File
> third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-04.html
> (right):
> 
>
https://codereview.chromium.org/1986563002/diff/240001/third_party/WebKit/Lay...
>
third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-04.html:33:
> if (window.internals)
> nit: How about this? Since, we don't want to write descriptions twice.
> function test_all(platform) {
>   if (platform)
>     window.internals.setEditingBehavior(platform);
>   test(test_sub, `${platform} run queryCommandState('subscript')`);
>   test(test_sup, `${platform} run queryCommandState('superscript')`);
> }
> 
> (windows.internal ? [''] : ['mac', 'win]).forEach(platform, test_all);

Question) are there any trybots that don't support window.internals?
If not, test_all('') cannot be run in Blink layout test.

[yosin_UTC9, 2016-05-30 08:10:03]

On 2016/05/30 at 07:30:30, joone.hur wrote:
> On 2016/05/30 06:43:42, Yosi_UTC9 wrote:
> >
https://codereview.chromium.org/1986563002/diff/240001/third_party/WebKit/Lay...
> > File
> > third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-04.html
> > (right):
> > 
> >
https://codereview.chromium.org/1986563002/diff/240001/third_party/WebKit/Lay...
> >
third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-04.html:33:
> > if (window.internals)
> > nit: How about this? Since, we don't want to write descriptions twice.
> > function test_all(platform) {
> >   if (platform)
> >     window.internals.setEditingBehavior(platform);
> >   test(test_sub, `${platform} run queryCommandState('subscript')`);
> >   test(test_sup, `${platform} run queryCommandState('superscript')`);
> > }
> > 
> > (windows.internal ? [''] : ['mac', 'win]).forEach(platform, test_all);
> 
> Question) are there any trybots that don't support window.internals?
No.
> If not, test_all('') cannot be run in Blink layout test.
test_all('') for opening in browser for debugging test itself, run this test on
Firefox, Edge, other than Blink, and at time of regression in the future.
So, writing test running other browsers help us.

[joone, 2016-05-30 09:16:18]

https://codereview.chromium.org/1986563002/diff/260001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-04.html
(right):

https://codereview.chromium.org/1986563002/diff/260001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/editing/execCommand/queryCommandState-04.html:35:
(window.internals ? ['mac', 'win'] : ['']).forEach(test_all);
==> (window.internals ? ['mac', 'win',''] : ['']).forEach(test_all);

How about this? We need to test the default editing behavior in layout test.

[yosin_UTC9, 2016-05-31 00:44:42]

lgtm

[yosin_UTC9, 2016-05-31 00:45:03]

Thanks for patient with long iteration!

[yosin_UTC9, 2016-05-31 00:45:06]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2016-05-31 00:45:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1986563002/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1986563002/260001

[commit-bot: I haz the power, 2016-05-31 00:48:34]

Description was changed from

==========
Apply vertical-align style of <sub> and <sup> to child elements.

If the selected element has <sub> or <sup> ancestor element,
apply the corresponding style(vertical-align) to it so that
document.queryCommandState() works fine on the selected element. 

<sub> and <sup> tags are represented with CSS vertical-align
property but they are not inherited. Therefore, we need to apply the 
property to child elements because <sub> and <sup> tag can have
child elements such as <i> or <b> tag.

BUG=582225
TEST=editing/execCommand/queryCommandState-04.html
     editing/execCommand/queryCommandState-04-mac.html
==========

to

==========
Apply vertical-align style of <sub> and <sup> to child elements.

If the selected element has <sub> or <sup> ancestor element,
apply the corresponding style(vertical-align) to it so that
document.queryCommandState() works fine on the selected element. 

<sub> and <sup> tags are represented with CSS vertical-align
property but they are not inherited. Therefore, we need to apply the 
property to child elements because <sub> and <sup> tag can have
child elements such as <i> or <b> tag.

BUG=582225
TEST=editing/execCommand/queryCommandState-04.html
     editing/execCommand/queryCommandState-04-mac.html
==========

[commit-bot: I haz the power, 2016-05-31 00:48:35]

Committed patchset #9 (id:260001)

[commit-bot: I haz the power, 2016-05-31 00:50:14]

Description was changed from

==========
Apply vertical-align style of <sub> and <sup> to child elements.

If the selected element has <sub> or <sup> ancestor element,
apply the corresponding style(vertical-align) to it so that
document.queryCommandState() works fine on the selected element. 

<sub> and <sup> tags are represented with CSS vertical-align
property but they are not inherited. Therefore, we need to apply the 
property to child elements because <sub> and <sup> tag can have
child elements such as <i> or <b> tag.

BUG=582225
TEST=editing/execCommand/queryCommandState-04.html
     editing/execCommand/queryCommandState-04-mac.html
==========

to

==========
Apply vertical-align style of <sub> and <sup> to child elements.

If the selected element has <sub> or <sup> ancestor element,
apply the corresponding style(vertical-align) to it so that
document.queryCommandState() works fine on the selected element.

<sub> and <sup> tags are represented with CSS vertical-align
property but they are not inherited. Therefore, we need to apply the
property to child elements because <sub> and <sup> tag can have
child elements such as <i> or <b> tag.

BUG=582225
TEST=editing/execCommand/queryCommandState-04.html
     editing/execCommand/queryCommandState-04-mac.html

Committed: https://crrev.com/dfe3d34820670521568ca7dfe681f701da94325b
Cr-Commit-Position: refs/heads/master@{#396763}
==========

[commit-bot: I haz the power, 2016-05-31 00:50:16]

Patchset 9 (id:??) landed as
https://crrev.com/dfe3d34820670521568ca7dfe681f701da94325b
Cr-Commit-Position: refs/heads/master@{#396763}

[joone, 2016-05-31 07:05:13]

On 2016/05/31 00:45:03, Yosi_UTC9 wrote:
> Thanks for patient with long iteration!

The change I suggested in my last comment was not applied to the commit so I
created another pull request:
https://codereview.chromium.org/2020183002/

Thank you for the reviews!