nsRuleNode::HasAuthorSpecifiedRules should handle inherited properties differently?

ASSIGNED
Assigned to

Status

()

Core
CSS Parsing and Computation
ASSIGNED
5 years ago
5 years ago

People

(Reporter: heycam, Assigned: heycam, NeedInfo)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
In nsRuleNode::HasAuthorSpecifiedRules there is this comment:

  // We need to be careful not to count styles covered up by user-important or
  // UA-important declarations.  But we do want to catch explicit inherit
  // styling in those and check our parent style context to see whether we have
  // user styling for those properties.  Note that we don't care here about
  // inheritance due to lack of a specified value, since all the properties we
  // care about are reset properties.

text-shadow is one of the properties we look at now, though, after bug 721750, and that's an inherited property.  Is there something in the loop below that that we need to change to handle the inherited property?  Otherwise, might it be possible that HasAuthorSpecifiedRules(NS_AUTHOR_SPECIFIED_TEXT_SHADOW) returns false when it should return true, if it finds a eCSSUnit_Null value for text-shadow?

This might only ever come up if we had say:

  div { text-shadow: 2px 2px 0 red; }
  span { letter-spacing: inherit; }

in an author or UA style sheet, and then with this content:

  <div><span>select me</span></div>

the selected text should paint with a text shadow but wouldn't.
(Assignee)

Comment 1

5 years ago
Created attachment 811489 [details] [diff] [review]
untested patch

Something like this?
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #811489 - Flags: feedback?(dbaron)
The original use of HasAuthorSpecifiedRules was to decide whether to disable native theme.  What does the current set of uses look like, especially for the callers that care about text-shadow?
Flags: needinfo?(cam)
(Assignee)

Comment 3

5 years ago
So the only use of HasAuthorSpecifiedRules that cares about text-shadow is here:

  https://mxr.mozilla.org/mozilla-central/source/layout/generic/nsTextFrame.cpp#3590

That's not to disable any native theming, but to allow a ::-moz-selection rule that doesn't have a text-shadow declaration to not clobber any text-shadow specified on the element without that pseudo.
Flags: needinfo?(cam) → needinfo?(dbaron)
You need to log in before you can comment on or make changes to this bug.