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

Status: RESOLVED FIXED

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

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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?

Comment 1

[Boris Zbarsky [:bzbarsky]]

That seems fine to me.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 1: add nsStyleContext::HasPseudoElementData

This patch is also needed for transitions.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 2: add pseudo element flags

This is only used by an assertion in the third patch, but I think it could be useful elsewhere at some point.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch 3: fix

Comment 5

[Boris Zbarsky [:bzbarsky]]

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.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(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.)

Comment 7

[Boris Zbarsky [:bzbarsky]]

> 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?

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.)

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://hg.mozilla.org/mozilla-central/rev/934047464748
http://hg.mozilla.org/mozilla-central/rev/4cc93ef5d614
http://hg.mozilla.org/mozilla-central/rev/135c87aa7a12