Don't assume line-height properties can't change between text and its container.

Don't assume line-height properties can't change between text and its container.

This assumption is totally wrong in a world with display: contents.

BUG=713019, 657748

[emilio, 2017-04-19 16:17:31]

The CQ bit was checked by ecobos@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-19 16:17:50]

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

[emilio, 2017-04-19 16:22:08]

The CQ bit was checked by ecobos@igalia.com to run a CQ dry run

[commit-bot: I haz the power, 2017-04-19 16:22:19]

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

[emilio, 2017-04-19 17:35:59]

ecobos@igalia.com changed reviewers:
+ eae@chromium.org

[emilio, 2017-04-19 17:36:00]

Hi Emil,

Could you take a look at this patch?

Thanks in advance :)

[commit-bot: I haz the power, 2017-04-19 18:32:04]

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

[commit-bot: I haz the power, 2017-04-19 18:32:05]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[eae, 2017-04-21 10:54:52]

This seems reasonable to me. LGTM

https://codereview.chromium.org/2826153002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/line/InlineFlowBox.cpp (right):

https://codereview.chromium.org/2826153002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/line/InlineFlowBox.cpp:97:
FlatTreeTraversal::Parent(*child.GetNode()) == parent.GetNode();
Can child.GetNode() ever be null? Might be worth adding a DCHECK here.

[kojii, 2017-08-02 07:06:00]

As I replied to the layout-dev@, this is reasonable, but this isn't the only
place we assume parent has the same fonts/vertical-align/etc, both in the
current layout code and in LayoutNG. We could fix them all, but I have mild
preference to create anonymous inline box in this case and keep the current
assumption.

Is this reasonable/possible?

[rune, 2017-08-02 07:42:08]

On 2017/08/02 07:06:00, kojii wrote:
> As I replied to the layout-dev@, this is reasonable, but this isn't the only
> place we assume parent has the same fonts/vertical-align/etc, both in the
> current layout code and in LayoutNG. We could fix them all, but I have mild
> preference to create anonymous inline box in this case and keep the current
> assumption.
> 
> Is this reasonable/possible?

I think that depends on how hard it would be to add/remove anonymous inline
boxes while rebuilding the layout tree. I would need to take a deeper look at
the layout tree building to answer that.

I'd expect we'd wrap subsequent text node children of display:contents elements
into an anonymous inline.

So that this (AAA and aaa being two separate text nodes):

  <div style="display:contents><div
style="display:contents">AAA,aaa<div>BBB</div>CCC<span>DDD</span></div>EEE</div>

will become:

  <anon-block>
    <anon-inline>AAA,aaa</anon-inline>
  </anon-block>
  <block>BBB</block>
  <anon-block>
   
<anon-inline>CCC</anon-inline><inline>DDD</inline><anon-inline>EEE</anon-inline>
  </anon-block>

where the anonymous inline for AAA, and CCC gets their fonts/colors etc from the
inner display:contents, and the anonymous inline for EEE gets relevant style
from the outer one.