Fix CSS outline property in flipped blocks Writing Mode (vertical-rl)

Fix CSS outline property in flipped blocks Writing Mode (vertical-rl)

This patch fixes CSS outline property for inline elements in flipped
blocks Writing Mode (vertical-rl.)

LayoutObject::addOutlineRects() returns rects in the object's
coordinate, which does not take flipped blocks into account if the
object is not a LayoutBox.

This patch calls LayoutObject::localToContainerRects(), which is
extracted from LayoutBoxModelObject.

BUG=491454
Committed: https://crrev.com/328e05af64df9ee8260c65e636d87d3c0c7cccac
Cr-Commit-Position: refs/heads/master@{#361297}

[kojii, 2015-11-19 08:11:51]

Description was changed from

==========
WIP: outline-vrl

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

to

==========
WIP: outline-vrl

BUG=491454
==========

[kojii, 2015-11-23 07:43:15]

Description was changed from

==========
WIP: outline-vrl

BUG=491454
==========

to

==========
Fix CSS outline property in flipped blocks Writing Mode (vertical-rl)

This patch fixes CSS outline property for inline elements in flipped
blocks Writing Mode (vertical-rl.)

LayoutObject::addOutlineRects() returns rects in the object's
coordinate, which does not take flipped blocks into account if the
object is not a LayoutBox.

This patch calls LayoutObject::localToContainerRects(), which is
extracted from LayoutBoxModelObject.

BUG=491454
==========

[kojii, 2015-11-23 07:44:03]

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

[kojii, 2015-11-23 07:44:04]

PTAL.

[wkorman, 2015-11-23 23:25:15]

wkorman@chromium.org changed reviewers:
+ wkorman@chromium.org

[wkorman, 2015-11-23 23:25:16]

Add a layout test to cover this case?

+wangxianzhu in case he has any comments as I think he worked on focus/outline
code recently.

[Xianzhu, 2015-11-23 23:37:07]

Could you fix LayoutObject::addOutlineRects() to take flipped blocks into
account for all objects?

On 2015/11/23 23:25:16, wkorman wrote:
> Add a layout test to cover this case?

imported/csswg-test/css-writing-modes-3/outline-inline-vrl-006.html seems to
cover this.

> +wangxianzhu in case he has any comments as I think he worked on focus/outline
> code recently.

[eae, 2015-11-24 01:34:22]

LGTM

[kojii, 2015-11-24 02:20:58]

On 2015/11/23 23:37:07, Xianzhu wrote:
> Could you fix LayoutObject::addOutlineRects() to take flipped blocks into
> account for all objects?

Tried that, but it flipped twice in some cases and gave up, please see PS2. The
current logic is consistent from one point of view that it knows itself, and
since flipping occurs in blocks, inline is not flipped.

wkorman@ is working on refactoring flipped blocks to do less flipping, hopefully
once for all, so that should clean things up. I just wanted to inform him that
this was not working so that he shouldn't wonder whether he regressed or
accidentally fixed something.

Does that look ok to you and to wkorman@?

> On 2015/11/23 23:25:16, wkorman wrote:
> > Add a layout test to cover this case?
> 
> imported/csswg-test/css-writing-modes-3/outline-inline-vrl-006.html seems to
> cover this.

Sorry not to mention this, the flow of this bug is that tests were written in
W3C, imported and marked as failure, then fix to remove the failure. Following
tests were imported:
outline-inline-block-vrl-006.html (pass today, still pass after patch)
outline-inline-vlr-006.html (pass today, still pass after patch)
outline-inline-vrl-006.html (fail today, fixed in this patch)
and there are some more manual tests in W3C (we don't import manual tests
today.)

[wkorman, 2015-11-24 02:47:58]

lgtm

[wkorman, 2015-11-24 02:51:20]

On 2015/11/24 at 02:20:58, kojii wrote:
> On 2015/11/23 23:37:07, Xianzhu wrote:
> > Could you fix LayoutObject::addOutlineRects() to take flipped blocks into
> > account for all objects?
> 
> Tried that, but it flipped twice in some cases and gave up, please see PS2.
The current logic is consistent from one point of view that it knows itself, and
since flipping occurs in blocks, inline is not flipped.
> 
> wkorman@ is working on refactoring flipped blocks to do less flipping,
hopefully once for all, so that should clean things up. I just wanted to inform
him that this was not working so that he shouldn't wonder whether he regressed
or accidentally fixed something.
> 
> Does that look ok to you and to wkorman@?
> 
> > On 2015/11/23 23:25:16, wkorman wrote:
> > > Add a layout test to cover this case?
> > 
> > imported/csswg-test/css-writing-modes-3/outline-inline-vrl-006.html seems to
> > cover this.
> 
> Sorry not to mention this, the flow of this bug is that tests were written in
W3C, imported and marked as failure, then fix to remove the failure. Following
tests were imported:
> outline-inline-block-vrl-006.html (pass today, still pass after patch)
> outline-inline-vlr-006.html (pass today, still pass after patch)
> outline-inline-vrl-006.html (fail today, fixed in this patch)
> and there are some more manual tests in W3C (we don't import manual tests
today.)

Sounds good to me, and thanks for the explanation on the tests, it makes sense.

[kojii, 2015-11-24 05:24:19]

The CQ bit was checked by kojii@chromium.org

[commit-bot: I haz the power, 2015-11-24 05:25:31]

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

[commit-bot: I haz the power, 2015-11-24 06:54:28]

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

[commit-bot: I haz the power, 2015-11-24 06:54:29]

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

[kojii, 2015-11-24 07:45:09]

The CQ bit was checked by kojii@chromium.org

[commit-bot: I haz the power, 2015-11-24 07:46:04]

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

[commit-bot: I haz the power, 2015-11-24 09:03:26]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-11-24 09:04:55]

Patchset 5 (id:??) landed as
https://crrev.com/328e05af64df9ee8260c65e636d87d3c0c7cccac
Cr-Commit-Position: refs/heads/master@{#361297}