Closed Bug 1428339 Opened 7 years ago Closed 7 years ago

Make mapped attribute resolution not require a pres context.

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

Part of bug 1418159.
Comment on attachment 8940198 [details] Bug 1428339: Make attribute mapping work without a pres context. https://reviewboard.mozilla.org/r/210514/#review216550 ::: dom/base/nsMappedAttributes.h:78 (Diff revision 1) > const nsAttrName* GetExistingAttrNameFromQName(const nsAString& aName) const; > int32_t IndexOfAttr(nsAtom* aLocalName) const; > > // Apply the contained mapper to the contained set of servo rules, > // unless the servo rules have already been initialized. > - void LazilyResolveServoDeclaration(nsPresContext* aPresContext); > + void LazilyResolveServoDeclaration(nsIDocument*); I don't think it's local style to leave out argument names like this. ::: dom/html/HTMLBodyElement.cpp:225 (Diff revision 1) > - if (!aData->PropertyIsSet(eCSSProperty_color) && > + if (!aData->PropertyIsSet(eCSSProperty_color)) { > - aData->PresContext()->UseDocumentColors()) { > // color: color > nscolor color; > const nsAttrValue* value = aAttributes->GetAttr(nsGkAtoms::text); > - if (value && value->GetColorValue(color)) > + if (value && value->GetColorValue(color) && !aData->ShouldIgnoreColors()) { Why move the ShouldIgnoreColors() call into here? It seems OK where it (well, UseDocumentColors) is. ::: layout/style/ServoSpecifiedValues.h:30 (Diff revision 1) > + ; > +#undef STYLE_STRUCT > + > public: > - ServoSpecifiedValues(nsPresContext* aContext, > - RawServoDeclarationBlock* aDecl); > + ServoSpecifiedValues(nsIDocument* aDocument, RawServoDeclarationBlock* aDecl) > + : GenericSpecifiedValues(StyleBackendType::Servo, aDocument, ALL_SIDS) I see it's just moving code, but you can use NS_STYLE_INHERIT_MASK instead of ALL_SIDS. ::: layout/style/nsRuleData.h:194 (Diff revision 1) > > void SetFontFamily(const nsString& aValue); > void SetTextDecorationColorOverride(); > void SetBackgroundImage(nsAttrValue& aValue); > > + nsPresContext* const mPresContext; What do we need to store mPresContext for? Is it just the UseDocumentColors() call? If so, we can grab it with mStyleContext->PresContext() rather than storing it, so just add: nsPresContext* PresContext() { return mStyleContext->PresContext(); } Also, the nsRuleData constructor doesn't really need to take aContext and aStyleContext, since again we can get the former off the latter.
Attachment #8940198 - Flags: review?(cam) → review+
Comment on attachment 8940198 [details] Bug 1428339: Make attribute mapping work without a pres context. https://reviewboard.mozilla.org/r/210514/#review216550 > What do we need to store mPresContext for? Is it just the UseDocumentColors() call? If so, we can grab it with mStyleContext->PresContext() rather than storing it, so just add: > > nsPresContext* PresContext() { return mStyleContext->PresContext(); } > > Also, the nsRuleData constructor doesn't really need to take aContext and aStyleContext, since again we can get the former off the latter. It's used in a couple more places, so I've kept it because some of them looked somewhat hot. But I've derived mPresContext from mStyleContext.
Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/9ff344df5f64 Make attribute mapping work without a pres context. r=heycam
Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/b446fed39f0f followup: Add missing include that busts some builds on a CLOSED TREE. r=me
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
This change should help fixing bug 1400771.
Blocks: 1400771
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: