Convert editing/deleting/delete-ligature-003.html to use w3c test harness

Convert editing/deleting/delete-ligature-003.html to use w3c test harness

This patch converts "editing/deleting/delete-ligature-003.html" to use
w3c test harness to simplify the code for improving code health.

BUG=n/a
TEST=LayoutTests/editing/deleting/delete-ligature-003.html

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

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

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

[commit-bot: I haz the power, 2016-07-20 05:01:31]

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

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

Description was changed from

==========
2016-07-20T13:58:12

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

to

==========
Convert editing/deleting/delete-ligature-003.html to use w3c test harness

This patch converts "editing/deleting/delete-ligature-003.html" to use
w3c test harness to simplify the code for improving code health.

BUG=n/a
TEST=LayoutTests/editing/deleting/delete-ligature-003.html
==========

[commit-bot: I haz the power, 2016-07-20 06:20:31]

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

[commit-bot: I haz the power, 2016-07-20 06:20:32]

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2016-07-20 07:10:16]

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

[commit-bot: I haz the power, 2016-07-20 07:10:32]

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

[yosin_UTC9, 2016-07-20 07:37:38]

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

[commit-bot: I haz the power, 2016-07-20 07:38:01]

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

[yosin_UTC9, 2016-07-20 08:04:56]

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

[yosin_UTC9, 2016-07-20 08:04:57]

PTAL

[commit-bot: I haz the power, 2016-07-20 08:53:59]

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

[commit-bot: I haz the power, 2016-07-20 08:54:00]

Dry run: This issue passed the CQ dry run.

[yoichio, 2016-07-21 01:49:17]

https://codereview.chromium.org/2163163002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/editing/deleting/delete-ligature-003.html
(right):

https://codereview.chromium.org/2163163002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/deleting/delete-ligature-003.html:11:
'This test requires window.internals');
You can put these asserts out of forEach

https://codereview.chromium.org/2163163002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/deleting/delete-ligature-003.html:20:
platform === 'mac'
if mac is special case, could you make another assert_selection 
for mac?

[yosin_UTC9, 2016-07-21 02:20:05]

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

[yosin_UTC9, 2016-07-21 02:20:23]

PTAL

https://codereview.chromium.org/2163163002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/editing/deleting/delete-ligature-003.html
(right):

https://codereview.chromium.org/2163163002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/deleting/delete-ligature-003.html:11:
'This test requires window.internals');
On 2016/07/21 at 01:49:17, yoichio wrote:
> You can put these asserts out of forEach
Done.

https://codereview.chromium.org/2163163002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/deleting/delete-ligature-003.html:20:
platform === 'mac'
On 2016/07/21 at 01:49:17, yoichio wrote:
> if mac is special case, could you make another assert_selection 
> for mac?

I don't think it is worth to do.

If we move "mac" to another assertion it could write as:
    function testIt(platform, expectedResult)
    {
        assert_selection(
            '<div contenteditable>\u0E27\u0E31|</div>',
            selection => {
                internals.settings.setEditingBehavior(platform);
                selection.document.defaultView.focus();
                eventSender.keyDown('Backspace', null);
                selection.document.execCommand('undo');
            },
            expectedResult,
            `${platform}: Undo of backspace key on ligature U+0E27 and U+0E31`);
    }

    ['win', 'unix', 'android'].forEach(platform =>
        testIt(platform, '<div contenteditable>\u0E27\u0E31|</div>');
    testIt('mac', '<div contenteditable>\u0E27|\u0E31^</div>');

CONS are:
 - expected result is specified far from input and action.
 - testIt() introduces one additional step to read code.

"platform === 'mac' ? expecationOfMac : expecationOfOther" is more concise and
easier to read.

[commit-bot: I haz the power, 2016-07-21 02:20:35]

Dry run: 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-21 04:10:56]

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

[commit-bot: I haz the power, 2016-07-21 04:10:57]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[yoichio, 2016-07-21 04:52:11]

https://codereview.chromium.org/2163163002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/editing/deleting/delete-ligature-003.html
(right):

https://codereview.chromium.org/2163163002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/deleting/delete-ligature-003.html:20:
platform === 'mac'
On 2016/07/21 02:20:23, Yosi_UTC9 wrote:
> On 2016/07/21 at 01:49:17, yoichio wrote:
> > if mac is special case, could you make another assert_selection 
> > for mac?
> 
> I don't think it is worth to do.
> 
> If we move "mac" to another assertion it could write as:
>     function testIt(platform, expectedResult)
>     {
>         assert_selection(
>             '<div contenteditable>\u0E27\u0E31|</div>',
>             selection => {
>                 internals.settings.setEditingBehavior(platform);
>                 selection.document.defaultView.focus();
>                 eventSender.keyDown('Backspace', null);
>                 selection.document.execCommand('undo');
>             },
>             expectedResult,
>             `${platform}: Undo of backspace key on ligature U+0E27 and
U+0E31`);
>     }
> 
>     ['win', 'unix', 'android'].forEach(platform =>
>         testIt(platform, '<div contenteditable>\u0E27\u0E31|</div>');
>     testIt('mac', '<div contenteditable>\u0E27|\u0E31^</div>');
> 
> CONS are:
>  - expected result is specified far from input and action.
>  - testIt() introduces one additional step to read code.
> 
> "platform === 'mac' ? expecationOfMac : expecationOfOther" is more concise and
> easier to read. 
> 
I just suggest
['win', 'unix', 'android'].forEach(  assert_selection(
            '<div contenteditable>\u0E27\u0E31|</div>',
            selection => {
                internals.settings.setEditingBehavior(platform);
                selection.document.defaultView.focus();
                eventSender.keyDown('Backspace', null);
                selection.document.execCommand('undo');
            },
            '<div contenteditable>\u0E27\u0E31|</div>',
            `${platform}: Undo of backspace key on ligature U+0E27 and U+0E31`);

 assert_selection(
            '<div contenteditable>\u0E27\u0E31|</div>',
            selection => {
                internals.settings.setEditingBehavior('mac');
                selection.document.defaultView.focus();
                eventSender.keyDown('Backspace', null);
                selection.document.execCommand('undo');
            },
            '<div contenteditable>\u0E27|\u0E31</div>',

[yosin_UTC9, 2016-07-21 05:42:20]

On 2016/07/21 at 04:52:11, yoichio wrote:
>
https://codereview.chromium.org/2163163002/diff/40001/third_party/WebKit/Layo...
> File third_party/WebKit/LayoutTests/editing/deleting/delete-ligature-003.html
(right):
> 
>
https://codereview.chromium.org/2163163002/diff/40001/third_party/WebKit/Layo...
> third_party/WebKit/LayoutTests/editing/deleting/delete-ligature-003.html:20:
platform === 'mac'
> On 2016/07/21 02:20:23, Yosi_UTC9 wrote:
> > On 2016/07/21 at 01:49:17, yoichio wrote:
> > > if mac is special case, could you make another assert_selection 
> > > for mac?
> > 
> > I don't think it is worth to do.
> > 
> > If we move "mac" to another assertion it could write as:
> >     function testIt(platform, expectedResult)
> >     {
> >         assert_selection(
> >             '<div contenteditable>\u0E27\u0E31|</div>',
> >             selection => {
> >                 internals.settings.setEditingBehavior(platform);
> >                 selection.document.defaultView.focus();
> >                 eventSender.keyDown('Backspace', null);
> >                 selection.document.execCommand('undo');
> >             },
> >             expectedResult,
> >             `${platform}: Undo of backspace key on ligature U+0E27 and
U+0E31`);
> >     }
> > 
> >     ['win', 'unix', 'android'].forEach(platform =>
> >         testIt(platform, '<div contenteditable>\u0E27\u0E31|</div>');
> >     testIt('mac', '<div contenteditable>\u0E27|\u0E31^</div>');
> > 
> > CONS are:
> >  - expected result is specified far from input and action.
> >  - testIt() introduces one additional step to read code.
> > 
> > "platform === 'mac' ? expecationOfMac : expecationOfOther" is more concise
and
> > easier to read. 
> > 
> I just suggest
> ['win', 'unix', 'android'].forEach(  assert_selection(
>             '<div contenteditable>\u0E27\u0E31|</div>',
>             selection => {
>                 internals.settings.setEditingBehavior(platform);
>                 selection.document.defaultView.focus();
>                 eventSender.keyDown('Backspace', null);
>                 selection.document.execCommand('undo');
>             },
>             '<div contenteditable>\u0E27\u0E31|</div>',
>             `${platform}: Undo of backspace key on ligature U+0E27 and
U+0E31`);
> 
>  assert_selection(
>             '<div contenteditable>\u0E27\u0E31|</div>',
>             selection => {
>                 internals.settings.setEditingBehavior('mac');
>                 selection.document.defaultView.focus();
>                 eventSender.keyDown('Backspace', null);
>                 selection.document.execCommand('undo');
>             },
>             '<div contenteditable>\u0E27|\u0E31</div>',

We should avoid copy-paste in the code. When we do your suggestion, it is
confusing
and hard to maintain.

When this test is failed in the future, we need to compare both input and
actions
then we realize both inputs and sample are same but result is different.

[yoichio, 2016-07-21 06:46:12]

lgtm

[yosin_UTC9, 2016-07-21 06:46:37]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2016-07-21 06:46: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-21 07:49:38]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-21 07:51:27]

Description was changed from

==========
Convert editing/deleting/delete-ligature-003.html to use w3c test harness

This patch converts "editing/deleting/delete-ligature-003.html" to use
w3c test harness to simplify the code for improving code health.

BUG=n/a
TEST=LayoutTests/editing/deleting/delete-ligature-003.html
==========

to

==========
Convert editing/deleting/delete-ligature-003.html to use w3c test harness

This patch converts "editing/deleting/delete-ligature-003.html" to use
w3c test harness to simplify the code for improving code health.

BUG=n/a
TEST=LayoutTests/editing/deleting/delete-ligature-003.html

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

[commit-bot: I haz the power, 2016-07-21 07:51:27]

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