Boris Zbarsky [:bzbarsky] Assignee
	Description • 8 years ago

Loading https://drafts.csswg.org/css-values-3/ hits:

Assertion failure: aChild->GetProperty(nsGkAtoms::restylableAnonymousNode) (Someone passed native anonymous content directly into frame construction.  Stop doing that!)

aChild is a scrollcorner for a scrollframe corresponding to a <details> element.  I need to figure out why we're trying to reframe such a thing and whether we should really be doing that.

	David Baron :dbaron:
	Comment 1 • 8 years ago

(This was split out from bug 1250725.)

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 2 • 8 years ago

We seem to have an undisplayed node for the scrollcorner.  We land under ElementRestyler::RestyleUndisplayedNodes with aDisplay set to NS_STYLE_DISPLAY_NONE (called from mozilla::ElementRestyler::DoRestyleUndisplayedDescendants for the <details> element).  In particular, we have undisplayed nodes for both a scrollbar and a scrollcorner.  The style context stored in the undisplayed node has NS_STYLE_DISPLAY_BOX as mDisplay for both.  We resolve a new style context and that _also_ has NS_STYLE_DISPLAY_BOX.  This does not equal aDisplay, so we try to reframe.

It's a bit weird that we try to reframe even though the display value did not change from the thing we had _last_ time...

Anyway, we already flag scrollbar as being reframeable for totally bogus-ish reasons (theme changes); but the scrollcorner is not thus flagged and hence asserts.

The reason the scrollcorner is undisplayed is that we have this special case for details in nsCSSFrameConstructor::AddFrameConstructionItemsInternal:

  // When constructing a child of a non-open <details>, create only the frame
  // for the main <summary> element, and skip other elements.
  auto* details = HTMLDetailsElement::FromContentOrNull(parent);
  if (details && details->IsDetailsEnabled() && !details->Open()) {
    auto* summary = HTMLSummaryElement::FromContentOrNull(aContent);
    if (!summary || !summary->IsMainSummary()) {
      SetAsUndisplayedContent(aState, aItems, aContent, styleContext,
                              isGeneratedContent);
      return;
    }
  }

Applying that to the scrollbar/details of the scrollframe seems bogus to me.  In particular, consider this testcase:

  <details style="display: block; overflow: scroll; width: 200px; height: 200px;
                  border: 1px solid black">
    <summary style="display: block; width: 300px; height: 300px;
                    border: 2px solid purple">Some summary</summary>
    The details
  </details>

Should this show the scrollbar even while the details is not open?  Seems to be like it should, but we currently do not.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 5 • 8 years ago

Comment on attachment 8784598 [details] [diff] [review]
Don't suppress the scroll bits of a non-open <details> that has the appropriate overflow styles

No, this is wrong, because it lets through ::before and ::after....

	Ting-Yu Lin [:TYLin] (UTC-8)
	Comment 7 • 8 years ago

Comment on attachment 8784652 [details] [diff] [review]
Don't suppress the scroll bits of a non-open <details> that has the appropriate overflow styles

Review of attachment 8784652 [details] [diff] [review]:
-----------------------------------------------------------------

Yes. We should display the scrollbar for details with overflow:scroll style even when the details is not open.

This patch looks good to me.

	Pulsebot
	Comment 8 • 8 years ago

Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/75e8d28abce1
Don't suppress the scroll bits of a non-open <details> that has the appropriate overflow styles.  r=TYLin

	Ryan VanderMeulen [:RyanVM]
	Comment 9 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/75e8d28abce1