Open Bug 1252709 Opened 7 years ago Updated 7 months ago

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

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

Tracking Status
firefox47 --- affected

People

(Reporter: xidorn, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(4 files, 1 obsolete file)

Attached file testcase (obsolete) —
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
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.
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.
Attached file testcase
This testcase is probably better.
Attachment #8725493 - Attachment is obsolete: true
This shows that the styles are correct, so perhaps it is an invalidation problem?
Mats do you mind taking a look?
Flags: needinfo?(mats)
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).
Flags: needinfo?(mats)
Attached patch hackSplinter Review
If I apply this patch that forces us into the middle branch for this case,
then the bug does not occur.
Component: Layout → CSS Parsing and Computation
Blocks: 1258012
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.
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.
You need to log in before you can comment on or make changes to this bug.