Fix selection handles in vertical writing modes.

Fix selection handles in vertical writing modes.

The selection handle positioning logic wasn't written with vertical
writing modes in mind and they end up attached to odd edges of
the selection rect (see screenshots on bug).  Position and
orient them more appropriately.

BUG=557529
Committed: https://crrev.com/faca4e859b4ae9fa38a7dd173eb6ff0562c8e3b4
Cr-Commit-Position: refs/heads/master@{#368448}

[aelias_OOO_until_Jul13, 2015-12-16 02:51:17]

Description was changed from

==========
Fix selection handles in vertical writing modes.

BUG=557529
==========

to

==========
Fix selection handles in vertical writing modes.

The selection handle positioning logic wasn't written with vertical
writing modes in mind and they end up attached to odd edges of
the selection rect (see screenshots on bug).  Position and
orient them more appropriately.

BUG=557529
==========

[aelias_OOO_until_Jul13, 2015-12-16 02:51:34]

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

[aelias_OOO_until_Jul13, 2015-12-16 02:51:51]

Hi kojii@, PTAL.

[kojii, 2015-12-16 17:30:58]

Thank you for working on this. I can comment on vertical flow, but I'm not very
familiar with composite selections and I'm not an owner. Do you have someone
appropriate to review for editing/frame/web?

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

https://codereview.chromium.org/1531473004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/RenderedPosition.cpp:248: bool vertical =
m_layoutObject->hasFlippedBlocksWritingMode() ||
m_layoutObject->style()->isFlippedLinesWritingMode();
I think |!m_layoutObject->isHorizontalWritingMode()| is better here.

https://codereview.chromium.org/1531473004/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/web/tests/data/composited_selection_bounds_vertical.html
(right):

https://codereview.chromium.org/1531473004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/data/composited_selection_bounds_vertical.html:10:
-webkit-writing-mode: vertical-rl;
Can you also add a test for |vertical-lr|?

[aelias_OOO_until_Jul13, 2016-01-08 02:23:55]

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

[aelias_OOO_until_Jul13, 2016-01-08 02:24:12]

All done, adding rbyers@ for Source/core review.

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

https://codereview.chromium.org/1531473004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/RenderedPosition.cpp:248: bool vertical =
m_layoutObject->hasFlippedBlocksWritingMode() ||
m_layoutObject->style()->isFlippedLinesWritingMode();
On 2015/12/16 at 17:30:58, kojii wrote:
> I think |!m_layoutObject->isHorizontalWritingMode()| is better here.

Done.  Note that I needed to do
m_layoutObject->style()->isHorizontalWritingMode() instead, as the bit on
LayoutObject itself isn't updated properly (expected?).

https://codereview.chromium.org/1531473004/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/web/tests/data/composited_selection_bounds_vertical.html
(right):

https://codereview.chromium.org/1531473004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/data/composited_selection_bounds_vertical.html:10:
-webkit-writing-mode: vertical-rl;
On 2015/12/16 at 17:30:58, kojii wrote:
> Can you also add a test for |vertical-lr|?

Done.

[kojii, 2016-01-08 04:16:51]

Non-owner LGTM, thank you for working on this again.

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

https://codereview.chromium.org/1531473004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/RenderedPosition.cpp:248: bool vertical =
m_layoutObject->hasFlippedBlocksWritingMode() ||
m_layoutObject->style()->isFlippedLinesWritingMode();
On 2016/01/08 02:24:12, aelias wrote:
> On 2015/12/16 at 17:30:58, kojii wrote:
> > I think |!m_layoutObject->isHorizontalWritingMode()| is better here.
> 
> Done.  Note that I needed to do
> m_layoutObject->style()->isHorizontalWritingMode() instead, as the bit on
> LayoutObject itself isn't updated properly (expected?).

It should have been set in LayoutBoxModelObject::updateFromStyle() which is
called from LayoutBoxModelObject::styleDidChange(), so it may happen if
m_layoutObject is non-box (inline etc.)

I think it's ok here, the purpose is to test horizontal/vertical rather than
flipped or not.

[Rick Byers, 2016-01-08 21:25:59]

Seems reasonable to me, thanks!  LGTM

[aelias_OOO_until_Jul13, 2016-01-08 21:37:49]

The CQ bit was checked by aelias@chromium.org

[commit-bot: I haz the power, 2016-01-08 21:38:35]

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

[commit-bot: I haz the power, 2016-01-08 22:39:11]

Committed patchset #3 (id:40001)

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

Description was changed from

==========
Fix selection handles in vertical writing modes.

The selection handle positioning logic wasn't written with vertical
writing modes in mind and they end up attached to odd edges of
the selection rect (see screenshots on bug).  Position and
orient them more appropriately.

BUG=557529
==========

to

==========
Fix selection handles in vertical writing modes.

The selection handle positioning logic wasn't written with vertical
writing modes in mind and they end up attached to odd edges of
the selection rect (see screenshots on bug).  Position and
orient them more appropriately.

BUG=557529

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

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

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