Closed Bug 505515 Opened 11 years ago Closed 11 years ago

getComputedStyle should return style as though element were not inside pseudo-element (:first-line/:first-letter)

Categories

(Core :: DOM: CSS Object Model, defect, P4)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.2b1

People

(Reporter: dbaron, Unassigned)

References

Details

Attachments

(3 files)

When an element is fully or partly inside a pseudo-element (:first-line or :first-letter, maybe ::selection in the future), getComputedStyle will, I think, generally return the style associated with the first part of the element.

I think this is the wrong behavior.

For consistency, I think when an element is partly or even fully inside a pseudo-element, we should give the computed style for what would happen if it were not in the pseudo-element.  (However, for layout-related "computed" style where getComputedStyle is really returning the used value rather than computed value, we'll have to just return stuff based on its real layout.)

Does this seem reasonable?
That seems fine to me.
This patch is also needed for transitions.
Attachment #393380 - Flags: review?(bzbarsky)
This is only used by an assertion in the third patch, but I think it could be useful elsewhere at some point.
Attachment #393381 - Flags: review?(bzbarsky)
Attached patch patch 3: fixSplinter Review
Attachment #393382 - Flags: review?(bzbarsky)
Attachment #393380 - Flags: review?(bzbarsky) → review+
Comment on attachment 393381 [details] [diff] [review]
patch 2: add pseudo element flags

>+// Separate from the array above so that we can have an array of
>+// nsStaticAtom, but with corresponding indices (so the i-th element of
>+// this array is the flags for the i-th pseudo-element in the previous
>+// array).

I'm not following this comment.  Why does this need to be a separate array, exactly?

>+// elements?  (Currently all such pseudo-elements are non-tree-like, but
>+// it's possible some might be introduced in the future that are
>+// tree-like but contain other elements.  If so, we should probably
>+// split this flag.)

I'm not following this either.  What does "tree-like" mean here?

Other than the comments, looks ok.
Attachment #393381 - Flags: review?(bzbarsky) → review+
Attachment #393382 - Flags: review?(bzbarsky) → review+
(In reply to comment #5)
> I'm not following this comment.  Why does this need to be a separate array,
> exactly?

Because we want to pass the other array to NS_RegisterStaticAtoms and nsAtomListUtils::IsMember.

> >+// elements?  (Currently all such pseudo-elements are non-tree-like, but
> >+// it's possible some might be introduced in the future that are
> >+// tree-like but contain other elements.  If so, we should probably
> >+// split this flag.)
> 
> I'm not following this either.  What does "tree-like" mean here?

It means things like some of the pseudo-elements proposed for XForms:  pseudo-elements that make a proper tree structure rather than interleaving the way :first-line, :first-letter, and ::selection do.  (In other words, pseudo-elements that act a lot like XBL.)
> Because we want to pass the other array to NS_RegisterStaticAtoms and
> nsAtomListUtils::IsMember.

Ah. I see.  So maybe document that directly?  And make the other comment clearer too?
I now have:

+// nsStaticAtom (to pass to NS_RegisterStaticAtoms and
+// nsAtomListUtils::IsMember), but with corresponding indices (so the
+// i-th element of this array is the flags for the i-th pseudo-element
+// in the previous array).

and:

+// Is this pseudo-element a pseudo-element that can contain other
+// elements?
+// (Currently pseudo-elements are either leaves of the tree (relative to
+// real elements) or they contain other elements in a non-tree-like
+// manner (i.e., like incorrectly-nested start and end tags).  It's
+// possible that in the future there might be container pseudo-elements
+// that form a properly nested tree structure.  If that happens, we
+// should probably split this flag into two.)
http://hg.mozilla.org/mozilla-central/rev/934047464748
http://hg.mozilla.org/mozilla-central/rev/4cc93ef5d614
http://hg.mozilla.org/mozilla-central/rev/135c87aa7a12
Status: NEW → RESOLVED
Closed: 11 years ago
Priority: -- → P4
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2b1
You need to log in before you can comment on or make changes to this bug.