Closed Bug 1349457 Opened 3 years ago Closed 3 years ago

stylo: eagerly style generated content just after it is created


(Core :: CSS Parsing and Computation, enhancement, P1)




Tracking Status
firefox55 --- fixed


(Reporter: heycam, Assigned: heycam)




(1 file)

When generated content is created, we need to eagerly style it.  Not doing this is the cause of layout/reftests/bugs/451168-1.html tripping the bug 1345695 assertion.
Comment on attachment 8849891 [details]
Bug 1349457 - stylo: Eagerly style generated content.

r=me with the |createdGeneratedContent| stuff removed, or a good reason why we should keep it.

::: layout/base/nsCSSFrameConstructor.cpp:1918
(Diff revision 2)
> +  if (createdGeneratedContent && aParentContent->IsStyledByServo()) {
> +    mPresShell->StyleSet()->AsServo()->StyleNewSubtree(container);
> +  }

Hm, shouldn't we style |container| unconditionally? I don't understand what case we're trying to optimize for with |createdGeneratedContent| and why it would be ok to skip |container| even if |createdGeneratedContent| were false.
Attachment #8849891 - Flags: review?(bobbyholley) → review+
Note that I'm assuming here that we only end up in this function if we are very likely to create actual generated content. Otherwise we'd be appending a lot of unnecessary XUL NAC to the DOM.
Assignee: nobody → cam
Priority: -- → P1
You are right that we get in here only if we have ::before/::after rules that could produce generated content.  It is possible for nsCSSFrameConstructor::CreateGeneratedContent to return null, either (a) if the |content: url()| didn't resolve to an image, or (b) the |content: attr(x)| didn't refer to an existing attribute on the element.  I assume such cases are rare, so we could style |container| unconditionally if you think it's not worth checking whether CreateGeneratedContent succeeded.
Yeah, let's keep it simple and unconditional.
Pushed by
stylo: Eagerly style generated content. r=bholley
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.