RenderFlexibleBox should be treated as a candidate for visible position.

RenderFlexibleBox should be treated as a candidate for visible position.

Visible selection in an editable area is recalculated whenever an inner value of div is changed.
If the inner value is empty, the visible selection recalculated with DeleteSelectionCommand::m_endingPostion
is not correct. Because RenderBlockFlow is only considered but not RenderFlexibleBox in Position::isCandidate
so that a calculated ending VisiblePosition for an editable div having ‘display : flex’ goes weird
and VisibleSelection is empty accordingly.

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

[changseok, 2015-02-03 08:50:31]

shivamidow@gmail.com changed reviewers:
+ eae@chromium.org, sigbjornf@opera.com

[changseok, 2015-02-03 08:50:32]

Please have a look. And would you trigger layout tests for me? I don't have a
power yet. :(

[eae, 2015-02-03 21:02:23]

LGTM

[changseok, 2015-02-03 23:35:15]

On 2015/02/03 21:02:23, eae wrote:
> LGTM

failures on mac:
http/tests/navigation/anchor-basic.html
failures on win:
inspector/sources/debugger/event-listener-breakpoints.html

Thanks for your review.
The failed tests on mac and win are not caused by this change. So let me commit
the patch.

[changseok, 2015-02-03 23:35:30]

The CQ bit was checked by shivamidow@gmail.com

[commit-bot: I haz the power, 2015-02-03 23:36:44]

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

[commit-bot: I haz the power, 2015-02-04 02:13:32]

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

[commit-bot: I haz the power, 2015-02-04 02:13:34]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/41146)

[changseok, 2015-02-04 05:42:43]

The CQ bit was checked by shivamidow@gmail.com

[commit-bot: I haz the power, 2015-02-04 05:43:41]

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

[commit-bot: I haz the power, 2015-02-04 08:15:19]

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

[commit-bot: I haz the power, 2015-02-04 08:15:19]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/41236)

[changseok, 2015-02-04 10:14:21]

The CQ bit was checked by shivamidow@gmail.com

[commit-bot: I haz the power, 2015-02-04 10:15:16]

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

[commit-bot: I haz the power, 2015-02-04 11:12:28]

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

[Manuel Rego, 2015-02-10 22:31:36]

rego@igalia.com changed reviewers:
+ rego@igalia.com

[Manuel Rego, 2015-02-10 22:31:37]

https://codereview.chromium.org/897633002/diff/1/Source/core/dom/Position.cpp
File Source/core/dom/Position.cpp (right):

https://codereview.chromium.org/897633002/diff/1/Source/core/dom/Position.cpp...
Source/core/dom/Position.cpp:858: if (renderer->isRenderBlockFlow() ||
renderer->isFlexibleBox()) {
I think this is failing for grid too. Could you fix it?

I think you can use here !renderer->isRenderBlockFlow() to cover both cases.

Thanks!

[Manuel Rego, 2015-02-10 22:43:24]

https://codereview.chromium.org/897633002/diff/1/Source/core/dom/Position.cpp
File Source/core/dom/Position.cpp (right):

https://codereview.chromium.org/897633002/diff/1/Source/core/dom/Position.cpp...
Source/core/dom/Position.cpp:858: if (renderer->isRenderBlockFlow() ||
renderer->isFlexibleBox()) {
On 2015/02/10 22:31:37, Manuel Rego wrote:
> I think this is failing for grid too. Could you fix it?
> 
> I think you can use here !renderer->isRenderBlockFlow() to cover both cases.

Mmmm, I've just realized that this won't make any sense. I'm wondering if we
should just add isRenderGrid or if this "if" is actually needed for something
else.

Anyway, this is failing in grid too.

[changseok, 2015-02-12 19:18:26]

On 2015/02/10 22:43:24, Manuel Rego wrote:
> https://codereview.chromium.org/897633002/diff/1/Source/core/dom/Position.cpp
> File Source/core/dom/Position.cpp (right):
> 
>
https://codereview.chromium.org/897633002/diff/1/Source/core/dom/Position.cpp...
> Source/core/dom/Position.cpp:858: if (renderer->isRenderBlockFlow() ||
> renderer->isFlexibleBox()) {
> On 2015/02/10 22:31:37, Manuel Rego wrote:
> > I think this is failing for grid too. Could you fix it?
> > 
> > I think you can use here !renderer->isRenderBlockFlow() to cover both cases.
> 
> Mmmm, I've just realized that this won't make any sense. I'm wondering if we
> should just add isRenderGrid or if this "if" is actually needed for something
> else.
> 
> Anyway, this is failing in grid too.

Yeah I saw it. I'm fixing it for webkit. http://webkit.org/b/141465
Let me apply the patch for webkit to blink accordingly. Thanks.

[eae, 2015-04-06 19:41:57]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1060113002/ by eae@chromium.org.

The reason for reverting is: Revert for now until the fix is ready. Will also be
reverted on release branch..