If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

implement nsIStyledContent::FromContent

RESOLVED FIXED

Status

()

Core
Layout
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: sicking, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Looking the attachment in bug 313207 we QI to nsIStyledContent a lot. These QIs
should be replacable with something like:

 static nsIStyledContent* FromContent(nsIContent *aContent)
 {
   if (aContent->IsContentOfType(eELEMENT))
     return NS_STATIC_CAST(nsIStyledContent*, aContent);
   return nsnull;
 }
Blocks: 313207
Without having any hard data to back this up but based on an LXR search, I would
guess that most of these QIs are from the AttributeChanged impl in the CSS frame
constructor (since I was instrumenting in bug 313207 in a build that already has
bryner's CSSStyleSheet patch to do exactly the sort of casting sicking
proposes).  So I was actually thinking that perhaps AttributeChanged should take
an nsIStyledContent, not an nsIContent.  The callers are basically all concrete
classes passing |this|.  And we only have attributes on elements, right?
That sounds like a great idea to do in addition to comment 0. Because yes, we
only have attributes on elements.
Really, we could do the same with ContentStatesChanged ContentAppended
ContentInserted and ContentRemoved
For content states changed, I believe we might call it on textnodes right now
(for hover, eg).

For those others, we could do it for the container, maybe.  But that would
involve some caller-side QIs since some places (content sink, eg) currently have
an nsIContent.
Is it time to rename nsIStyledContent to nsIElement yet, btw?  ;)
good point about the QIs. Though if we do rename nsIStyledContent to nsIElement
it would make sense to let the sinks deal with those rather then nsIContent.
That might be a big change with doubtfull win though, especially with the
"fast-cast"
Yeah.  More to the point, as things stand we use nsIStyledContent in the
following places (per lxr):

nsHTMLTableCellElement::WalkContentStyleRules (needs to walk style rules on the
   table)
nsXMLEventsListener::HandleEvent (calls GetID()).
nsCSSFrameConstructor::AttributeChanged (needs to call GetAttributeChangeHint).
style resolution (already uses an IsContentOfType check + cast).

That's it.  So changing the sinks would have no benefit -- no consumer of
ContentAppended/Inserted/Removed actually wants the nsIStyledContent interface.
GetID() should move to nsIContent anyway, that's where we have all the other
attribute stuff.
I'd rather GetID() went away, at least if we plan to do xml:id.
Depends on: 313968
nsIStyledContent is no more.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Comment 11

12 years ago
"Fixed" by bug 313968.
You need to log in before you can comment on or make changes to this bug.