Fix long-press image selection inside editables.

Fix long-press image selection inside editables.

This fixes the regression introduced in http://crrev.com/1668453002 that
contenteditable images were no longer selectable with long-press, and
also fixes the longer-standing issue that long-press selection only worked
on the left side of images.

I also removed unnecessary #if OS(ANDROID), which allows the long-press behavior
to work similarly on other touch platforms and lets us run the tests on
all platforms.

BUG=597506, 585020
Committed: https://crrev.com/1fa55a60390ac8fa93a336c666fe01c671534833
Cr-Commit-Position: refs/heads/master@{#385360}

[aelias_OOO_until_Jul13, 2016-03-31 03:36:47]

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

[commit-bot: I haz the power, 2016-03-31 03:37:17]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1845193002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1845193002/1

[commit-bot: I haz the power, 2016-03-31 04:37:29]

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

[commit-bot: I haz the power, 2016-03-31 04:37:29]

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

[aelias_OOO_until_Jul13, 2016-04-05 03:38:38]

Description was changed from

==========
Fix long-press image selection inside editables.

BUG=597506
==========

to

==========
Fix long-press image selection inside editables.

BUG=597506, 585020
==========

[aelias_OOO_until_Jul13, 2016-04-05 03:40:43]

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

[commit-bot: I haz the power, 2016-04-05 03:41:08]

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

[aelias_OOO_until_Jul13, 2016-04-05 03:42:52]

Description was changed from

==========
Fix long-press image selection inside editables.

BUG=597506, 585020
==========

to

==========
Fix long-press image selection inside editables.

This fixes the regression introduced in http://crrev.com/1668453002 that
contenteditable images were no longer selectable with long-press, and
also fixes the longer-standing issue that long-press selection only worked
on the left side of images.

I also removed unnecessary #if OS(ANDROID), which allows the long-press behavior
to work similarly on other touch platforms and lets us run the tests on
all platforms.

BUG=597506, 585020
==========

[aelias_OOO_until_Jul13, 2016-04-05 03:42:52]

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

[aelias_OOO_until_Jul13, 2016-04-05 03:43:35]

Hi yosin@, PTAL.

[aelias_OOO_until_Jul13, 2016-04-05 03:45:55]

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

[commit-bot: I haz the power, 2016-04-05 03:46:07]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1845193002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1845193002/40001

[commit-bot: I haz the power, 2016-04-05 04:52:50]

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

[commit-bot: I haz the power, 2016-04-05 04:52:51]

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

[aelias_OOO_until_Jul13, 2016-04-05 05:15:33]

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

[commit-bot: I haz the power, 2016-04-05 05:16:07]

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

[yosin_UTC9, 2016-04-05 05:16:45]

It seems some layout tests failed. At glance, there tests have no image. So,
this patch should not affect them.

Could you take look failed layout tests?

https://codereview.chromium.org/1845193002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/SelectionController.cpp (right):

https://codereview.chromium.org/1845193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/SelectionController.cpp:303: if
(result.image()) {
nit: we don't need to have "{}" here, since then-clause is one statement.

https://codereview.chromium.org/1845193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/SelectionController.cpp:316:
EphemeralRangeInFlatTree range = EphemeralRangeInFlatTree(newSelection.start(),
newSelection.end());
nit: |const EphmeralRangeInFlatTree&| to avoid unnecessary copy

https://codereview.chromium.org/1845193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/SelectionController.cpp:320: } else {
To avoid nesting, we can write:

} else if (newSelection.isRange()) {
  newSelection..appendTrailingWhitespace();
}

[aelias_OOO_until_Jul13, 2016-04-05 06:48:37]

https://codereview.chromium.org/1845193002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/SelectionController.cpp (right):

https://codereview.chromium.org/1845193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/SelectionController.cpp:303: if
(result.image()) {
On 2016/04/05 at 05:16:45, Yosi_UTC9 wrote:
> nit: we don't need to have "{}" here, since then-clause is one statement.

Done.

https://codereview.chromium.org/1845193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/SelectionController.cpp:316:
EphemeralRangeInFlatTree range = EphemeralRangeInFlatTree(newSelection.start(),
newSelection.end());
On 2016/04/05 at 05:16:45, Yosi_UTC9 wrote:
> nit: |const EphmeralRangeInFlatTree&| to avoid unnecessary copy

Switched to parens-on-local-variable-constructor syntax which is a more concise
equivalent.

https://codereview.chromium.org/1845193002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/SelectionController.cpp:320: } else {
On 2016/04/05 at 05:16:45, Yosi_UTC9 wrote:
> To avoid nesting, we can write:
> 
> } else if (newSelection.isRange()) {
>   newSelection..appendTrailingWhitespace();
> }

Switched this back to the former syntax anyway.

https://codereview.chromium.org/1845193002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/editing/SelectionController.cpp (right):

https://codereview.chromium.org/1845193002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/editing/SelectionController.cpp:290: void
SelectionController::selectClosestWordFromHitTestResult(const HitTestResult&
result, AppendTrailingWhitespace appendTrailingWhitespace, bool isLongPress)
The failing layout tests were due to the OS(ANDROID) removal, the
AppendWhitespace value wasn't enough to disambiguate the platforms after all.  I
added a new argument for this.

[yosin_UTC9, 2016-04-05 07:27:47]

https://codereview.chromium.org/1845193002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/editing/SelectionController.cpp (right):

https://codereview.chromium.org/1845193002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/editing/SelectionController.cpp:555:
selectClosestWordFromHitTestResult(hitTestResult,
AppendTrailingWhitespace::DontAppend, true);
Could you introduce enum class rather than |bool| parameter?
http://dev.chromium.org/blink/coding-style [names-enum-to-bool]

[aelias_OOO_until_Jul13, 2016-04-05 08:26:45]

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

[aelias_OOO_until_Jul13, 2016-04-05 08:26:53]

https://codereview.chromium.org/1845193002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/editing/SelectionController.cpp (right):

https://codereview.chromium.org/1845193002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/editing/SelectionController.cpp:555:
selectClosestWordFromHitTestResult(hitTestResult,
AppendTrailingWhitespace::DontAppend, true);
On 2016/04/05 at 07:27:46, Yosi_UTC9 wrote:
> Could you introduce enum class rather than |bool| parameter?
> http://dev.chromium.org/blink/coding-style [names-enum-to-bool]

Done.

[commit-bot: I haz the power, 2016-04-05 08:27:01]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1845193002/130001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1845193002/130001

[commit-bot: I haz the power, 2016-04-05 09:30:00]

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

[commit-bot: I haz the power, 2016-04-05 09:30:01]

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2016-04-06 01:06:40]

lgtm

Thanks for removing #if OS(ANDROID)!

[aelias_OOO_until_Jul13, 2016-04-06 01:15:10]

The CQ bit was checked by aelias@chromium.org

[commit-bot: I haz the power, 2016-04-06 01:15:43]

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

[commit-bot: I haz the power, 2016-04-06 01:21:42]

Committed patchset #8 (id:130001)

[commit-bot: I haz the power, 2016-04-06 01:23:37]

Description was changed from

==========
Fix long-press image selection inside editables.

This fixes the regression introduced in http://crrev.com/1668453002 that
contenteditable images were no longer selectable with long-press, and
also fixes the longer-standing issue that long-press selection only worked
on the left side of images.

I also removed unnecessary #if OS(ANDROID), which allows the long-press behavior
to work similarly on other touch platforms and lets us run the tests on
all platforms.

BUG=597506, 585020
==========

to

==========
Fix long-press image selection inside editables.

This fixes the regression introduced in http://crrev.com/1668453002 that
contenteditable images were no longer selectable with long-press, and
also fixes the longer-standing issue that long-press selection only worked
on the left side of images.

I also removed unnecessary #if OS(ANDROID), which allows the long-press behavior
to work similarly on other touch platforms and lets us run the tests on
all platforms.

BUG=597506, 585020

Committed: https://crrev.com/1fa55a60390ac8fa93a336c666fe01c671534833
Cr-Commit-Position: refs/heads/master@{#385360}
==========

[commit-bot: I haz the power, 2016-04-06 01:23:38]

Patchset 8 (id:??) landed as
https://crrev.com/1fa55a60390ac8fa93a336c666fe01c671534833
Cr-Commit-Position: refs/heads/master@{#385360}