1252709 - Changes to inherit property on element with "display: contents" aren't propagated to its children

Status: NEW

(Core :: CSS Parsing and Computation, defect)

Description

[Xidorn Quan [:xidorn] UTC+10]

Attachment: testcase

See the attachment. Both "test" should be green, however the first is left red until you select it or doing anything else trigger a paint.

It indicates that value change of inherited properties doesn't trigger the proper invalidation.

This was reported by a Microsoft guy in the www-style mailing list: https://lists.w3.org/Archives/Public/www-style/2016Mar/0020.html

Comment 1

[Cameron McCormack (:heycam)]

Seems hard to reproduce on my computer.  I only see red text occasionally.  I wonder if the styles are right but the invalidation didn't happen, or if the styles were computed incorrectly.

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

You can use other properties, e.g. those require reflow to take effect, that would probably have a better chance to reproduce. Any inherited property has this issue.

Comment 3

[Xidorn Quan [:xidorn] UTC+10]

Attachment: testcase

This testcase is probably better.

Comment 4

[Cameron McCormack (:heycam)]

Attachment: testcase with console.log

This shows that the styles are correct, so perhaps it is an invalidation problem?

Comment 5

[Cameron McCormack (:heycam)]

Mats do you mind taking a look?

Comment 6

[Mats Palmgren (inactive)]

Attachment: stack for text frame restyle

So it appears that we're restyling the text frame correctly, but we
ignore the change hint (NS_STYLE_HINT_REFLOW) from nsStyleFont::CalcDifference.
Here's the stack for that call.  After doing 'up' you can see the values used
before that call in this macro:
http://hg.mozilla.org/mozilla-central/annotate/9c5d494d0548/layout/style/nsStyleContext.cpp#l861
It appears that we're in the last branch there (line 891), and thus we will
ignore the returned 'difference' rather than updating 'hint' as we would
have done in the middle branch (on line 883).

Comment 7

[Mats Palmgren (inactive)]

Attachment: hack

If I apply this patch that forces us into the middle branch for this case,
then the bug does not occur.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

What happens when we process the style change for the 'display:contents' element?  My initial reaction is that this bug ought to be fixed there, not when we process the style change for its child.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I don't completely understand the display:contents-related code in ElementRestyler.  But my first impression is that it looks like we traverse to the children of a display:contents element directly from the restyling of the parent of the display:contents element (in RestyleContentChildren), rather than via the display:contents element.  I think this is probably a mistake.  A number of things in restyling depend on it operating recursively on the tree that we use for style inheritance.  (For example, is the AncestorPusher stuff right, or do we end up doing selector matching wrong?)

I also don't see any place where we do something with the nsChangeHint that results from the restyling of a display:contents element.  I think there needs to be something.

If we did the first change (of how we traverse the tree), I think we could then take the hints that we compute for a display:contents element and apply those hints (or at least the hints that are "HandledForDescendants") to the children of the display:contents element as additional hints passed in to the restyling process for those elements.