Closed Bug 898333 Opened 9 years ago Closed 8 years ago

make ElementRestyler::Restyle traverse tree through continuations


(Core :: CSS Parsing and Computation, defect)

Not set





(Reporter: dbaron, Assigned: dbaron)




(2 files)

As I suggested in bug 828312 comment 69, we need to change ElementRestyler::Restyle (to be refactored in bug 898329) so that the tree traversal works differently.

Currently ElementRestyler::Restyle does a normal depth-first traversal of the frame tree (with an exception for outer/inner tables, a.k.a. tables and table wrappers).  However, in bug 828312 I want to change the semantics of change hints so that we only do difference computation on the first continuation.  This, in turn, requires that we process the descendants of all continuations (and same-style parts of block-in-inline splits) inside the code where we process the first continuation.  In other words, this requires that we do a depth-first search as though continuations and same-style block-in-inline splits were merged back together.
Assignee: nobody → dbaron
This patch fundamentally makes three changes:

 (1) Change the way RestyleSelf is used so that it is called in a loop,
     over all of the same-style continuations and block-in-inline
     (I had a note here reminding myself:
       TODO: Don't set hints for descendants!
     but I know longer remember what it meant.)

 (2) Change the traversal of children to traverse all of the children of
     the items traversed in (1).

 (3) When traversing children, skip children that are continuations,
     since we already reached them in (1) and (2).

A later patch will change the RestyleSelf loop to copy for the later
continuations rather than repeating the work.  This will mean reverting
many of the RestyleSelf changes contained here.
Attachment #805489 - Flags: review?(bzbarsky)
Comment on attachment 805488 [details] [diff] [review]
, patch 1:  Restyle the :after pseudo-element after the content children.

Hmm.  So the order was what it was so that if we have to add/remove an ::after we don't have to waste time restyling the kids.

I guess that's a rare case anyway.  r=me
Attachment #805488 - Flags: review?(bzbarsky) → review+
It seemed necessary (at least for simplicity) in patch 2 (see the changes in patch 2 to the same code), and it was easy to split into its own patch.  (I thought about splitting up patch 2 more, but didn't see a nice way to create fully working intermediate states.)
Comment on attachment 805489 [details] [diff] [review]
, patch 2:  Change RestyleManager::Restyle's tree traversal to reach next-continuations (and same-display block-in-inline siblings) from their prev-continuation rather than from their parent.

>     but I know longer remember what it meant.)

"no longer"?

>+                               nsIFrame* aContinuation, // TEMPORARY

Where will it get removed?  Same for the other temporary bits.

>+ * with the same style) all of its next continuations with the same

Comma before "all", please.

Attachment #805489 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky [:bz] from comment #8)
> >+                               nsIFrame* aContinuation, // TEMPORARY
> Where will it get removed?  Same for the other temporary bits.

Some will get removed in bug 828312 patch 11 (attachment 806310 [details] [diff] [review]), and the rest will, I hope, get removed in a followup bug to restructure things to set the style context directly on all the continuations in a single RestyleSelf call instead of having to invoke RestyleSelf for each continuation.  (Though that followup might require further followups to fix the same pattern in ReParentStyleContext.)
I filed bug 918064 on that followup, and annotated the TEMPORARY comments with when they're expected to be removed (though only one remains after bug 828312, and it points to bug 918064).
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Depends on: 927267
Depends on: 945105
You need to log in before you can comment on or make changes to this bug.