Closed Bug 1198607 Opened 8 years ago Closed 8 years ago

Returning nsChangeHint_UpdateOverflow from nsStyleText::CalcDifference() causes "###!!! ASSERTION: Shouldn't be trying to restyle non-elements directly, except if it's a display:contents child"

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED INVALID
Tracking Status
firefox43 --- affected

People

(Reporter: dholbert, Unassigned)

References

Details

STR:
 1. Adjust nsStyleText::CalcDifference() to return
      nsChangeHint_UpdateOverflow | nsChangeHint_SchedulePaint
    instead of NS_STYLE_HINT_REFLOW  when mTextShadow differs.
  (I'll attach a patch to do this.)
  (Be sure to also update nsStyleText::MaxDifference(), too.)

 2. Compile a debug build with this change.

 3. Visit attachment 593939 [details] (from bug 723669)

ACTUAL RESULTS: The testcase renders correctly, but it triggers this assertion on load:
{
 ###!!! ASSERTION: Shouldn't be trying to restyle non-elements directly, except if it's a display:contents child: '!aContent || aContent->IsElement() || (aFrame && aContent->GetParent() && aFrame->PresContext()->FrameManager()-> GetDisplayContentsStyleFor(aContent->GetParent()))', file layout/base/nsStyleChangeList.cpp, line 69
}

The frame being restyled is for a text node (i.e. a non-null content node which is not an element, hence the assertion). We're trying to restyle it to update its overflow region, I believe.

This assertion dates back to bug 367650 from bz.  (It's been modified a bit, but the spirit of the assertion remains the same.) I'm hoping that perhaps bz can clue me in on whether the assertion is actually telling me I'm doing something wrong (entirely likely), or if the assertion just needs changing. 

[CC'ing mats too, since he touched the assertion most recently for 'display:contents' & perhaps can comment]

The frame in question is for a text node (which is not an element), and we're trying to restyle it to update its overflow regions, I believe. I don't know if this is actually problematic (i.e. if there's some more code that needs changing for this to work properly), or if the assertion just needs adjusting to allow [certain?] restyles of text-nodes...
It's not valid for a CalcDifference hint to return a non-inherited hint for an inherited property.

You should use UpdateSubtreeOverflow instead of UpdateOverflow.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → INVALID
ni? dholbert to get a bug filed on fixing text-shadow hints if he doesn't want to reuse bug 1194480.
Flags: needinfo?(dholbert)
It also might make sense to make the assertion text point this out more directly.
(In reply to David Baron [:dbaron] ⌚UTC+9 [busy, returning November 2] from comment #2)
> ni? dholbert to get a bug filed on fixing text-shadow hints if he doesn't
> want to reuse bug 1194480.

I did end reusing bug 1194480 for this (fixing both text-shadow & box-shadow there) -- its patch made us update overflow regions (with UpdateSubtreeOverflow as dbaron suggested) instead of reflowing for "text-shadow" changes, as shown e.g. here:
 https://hg.mozilla.org/mozilla-central/rev/009f9286101b#l1.164

So I think this is addressed. Please tag me again if I'm misunderstanding, though.
Flags: needinfo?(dholbert)
You need to log in before you can comment on or make changes to this bug.