493123 - "ASSERTION: Scrollbars should have been propagated to the viewport" with stylesheet in XBL

Status: RESOLVED WORKSFORME

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! ASSERTION: Scrollbars should have been propagated to the viewport: '!display->IsScrollableOverflow() || state.mPresContext->IsPaginated() || propagatedScrollToViewport', file /Users/jruderman/central/layout/base/nsCSSFrameConstructor.cpp, line 2545

Related to bug 78070?

Comment 1

[Jesse Ruderman]

Has to be loaded locally due to interaction between XBL loading and Bugzilla caching.

Comment 2

[Boris Zbarsky [:bzbarsky]]

It's not obviously related. The issue here is that when we reset the binding back to #a, the LoadBindings() call we nuke the anonymous content we used to have from #b, and in particular remove the <style> element from the DOM.  This happens via the FlushStyleBindings call in nsXBLService::LoadBindings:

#0  nsHTMLStyleElement::UnbindFromTree
#1  0x135f13a9 in nsXBLBinding::UninstallAnonymousContent
#2  0x135f3198 in nsXBLBinding::ChangeDocument
#3  0x1360e811 in nsXBLService::FlushStyleBindings
#4  0x13612b73 in nsXBLService::LoadBindings

So after this point, that style no longer applies.  Key here is that #a was already loaded before, so the removal of #b is synchronous here.  See the code flow in LoadBindings.

Of course resolveStyle comes back false, since there are no stylesheet resources in the binding.  So we don't reresolve style at this point and keep thinking that we have overflow:hidden set on the root, when in fact we no longer do.

Obvious options: 
1) Unconditionally reresolve style when loading a binding.  We'd have to do
   this for at least root elements and bodies.  Might be cheap and simple
   enough if we do it for just those two.
2) Stop loading stylesheets for anonymous <style> and <link> elements.  XBL can
   already provide resource stylesheets if it wants (though those do get scoped
   to the bound element, I believe).

#1 is the compat-preserving thing; #2 is what I think might be the sane one...

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I guess I don't have a strong opinion either way. If performance costs are low it's always less risky to keep compat. However it's a pretty crazy thing to do to include stylesheets in the anonymous content. Though I think the xslt pretty-printer might be doing that, though we can always change it.

Comment 4

[Jesse Ruderman]

WFM.  bz, do you still want to make any of the changes you mentioned in comment 2?

Comment 5

[Jesse Ruderman]

Crashtest: http://hg.mozilla.org/mozilla-central/rev/e5dd52dd3cb5

Comment 6

[Boris Zbarsky [:bzbarsky]]

> bz, do you still want to make any of the changes you mentioned in comment 2?

Yes.  It should be pretty simple to write testcases that still show problems due to those issues (and in fact, I'm tempted to reopen this bug....).  Please file a followup?

Comment 7

[Boris Zbarsky [:bzbarsky]]

I filed bug 579976.