Closed Bug 390689 Opened 12 years ago Closed 12 years ago

[FIX]Audit style resolution style context parentage

Categories

(Core :: Layout, defect, P2)

x86
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla1.9beta1

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

Attachments

(1 file)

From bug 384649 comment 11:

I just checked all the callers of ResolveStyleFor(), and the following look
fishy:

* nsSplitterFrame::Init: this should be getting the right style parent
                         directly off its style context.
* nsMathMLmactionFrame::Init: same
* ReResolveStyleContext when reresolving the undisplayed content.  I'm just
  not sure how that works, given that it's keyed off the content, and various
  stacked anonymous boxes can share the same content pointer...  Do you happen
  to know?

For ResolveStyleForNonElement, the only issue I see is the
ReResolveStyleContext one.

For ResolvePseudoStyleFor resolving style for pseudo-elements (which is where
we might need to adjust the style context):
* nsBlockFrame::SetInitialChildList is covered by another patch you just
  reviewed.
* ReResolveStyleContext... I can't tell offhand.  :(

The Init() callers are easy to fix, but we need to sort out what the deal with ReResolveStyleContext is.
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9-
Whiteboard: [wanted-1.9]
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #280951 - Flags: superreview?(dbaron)
Attachment #280951 - Flags: review?(dbaron)
Priority: -- → P2
Summary: Audit style resolution style context parentage → [FIX]Audit style resolution style context parentage
Target Milestone: --- → mozilla1.9 M9
Comment on attachment 280951 [details] [diff] [review]
I think this fixes all the issues

r+sr+a1.9=dbaron.  But please file bugs on the "XXXbz" comments in nsSplitterFrame and nsMathMLmactionFrame -- and the first one should probably depend on fixing XUL stylistic attribute mapping (and probably be fixed around the same time)
Attachment #280951 - Flags: superreview?(dbaron)
Attachment #280951 - Flags: superreview+
Attachment #280951 - Flags: review?(dbaron)
Attachment #280951 - Flags: review+
Attachment #280951 - Flags: approval1.9+
I think we probably shouldn't have things with pseudos in the undisplayed map, but I'm not sure.  It seems sort of scary if we do.
> I think we probably shouldn't have things with pseudos in the undisplayed map,

That's not so much what I was worrying about.  I was worrying about, say, cases like this:

  <span style="display: table">
    x <span style="display:none"> y
  </span>

The undisplayed node should have the outer span as the style parent.  But there are anonymous table boxes (table-row-group, table-row, table-cell) that have the outer span as mContent, and we don't want to reresolve that undisplayed node with those parents.  Not that it matters much, since we largely just want to know whether the style is display:none or not and that won't depend on which of those we use as parent.  But just for correctness' sake.
Filed bug 398506 and bug 398505.  Checked in the patch.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Flags: wanted1.9+
Whiteboard: [wanted-1.9]
Product: Core → Core Graveyard
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.