Closed
Bug 1297064
Opened 8 years ago
Closed 8 years ago
stylo: Add bindings for filling nsStyleImageLayers
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: manishearth, Assigned: manishearth)
References
Details
Attachments
(1 file)
While we can do this directly from servo, we'd have to manually call copy constructors.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → manishearth
Status: NEW → ASSIGNED
Comment 2•8 years ago
|
||
mozreview-review |
Comment on attachment 8783528 [details] Bug 1297064 - stylo: Add bindings for filling nsStyleImageLayers lists; https://reviewboard.mozilla.org/r/73320/#review71102 Not a formal review at all, just passed by and saw the style nits :P ::: layout/style/ServoBindings.cpp:281 (Diff revision 1) > } > return attr->GetServoCSSDeclarationValue(); > } > > +void > +Gecko_FillAllBackgroundLists(nsStyleImageLayers* aLayers, uint32_t aMaxLen) { nit: brace in its own line. ::: layout/style/nsRuleNode.cpp:7189 (Diff revision 1) > aLayers[destLayer].mPosition.*aResultLocation = > aLayers[sourceLayer].mPosition.*aResultLocation; > } > } > > +void FillAllBackgroundLists(nsStyleImageLayers& aImage, nit: return type in its own line, brace too.
Comment hidden (mozreview-request) |
Comment 4•8 years ago
|
||
mozreview-review |
Comment on attachment 8783528 [details] Bug 1297064 - stylo: Add bindings for filling nsStyleImageLayers lists; https://reviewboard.mozilla.org/r/73320/#review72334 ::: layout/style/ServoBindings.h:244 (Diff revision 2) > void Gecko_CopyClipPathValueFrom(mozilla::StyleClipPath* dst, const mozilla::StyleClipPath* src); > > void Gecko_DestroyClipPath(mozilla::StyleClipPath* clip); > mozilla::StyleBasicShape* Gecko_NewBasicShape(mozilla::StyleBasicShapeType type); > > +void Gecko_FillAllBackgroundLists(nsStyleImageLayers* layers, uint32_t maxLen); Nit: "max_len" so we generate a Rust friendly argument name? ::: layout/style/nsRuleNode.h:1070 (Diff revision 2) > +void FillAllBackgroundLists(nsStyleImageLayers& aLayers, > + uint32_t aMaxItemCount); Please make this a public static method of nsRuleNode. (We should avoid having global namespace functions. Putting it in |namespace mozilla| would be OK too, but I prefer sticking it on nsRuleNode so that callers can see where it's coming from / what it's related to.) ::: layout/style/nsRuleNode.cpp:7192 (Diff revision 2) > } > > +void > +FillAllBackgroundLists(nsStyleImageLayers& aImage, uint32_t aMaxItemCount) > +{ > + // Delete any extra items. We need to keep layers in which any Two space indent.
Attachment #8783528 -
Flags: review?(cam) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Pushed by manishearth@gmail.com: https://hg.mozilla.org/integration/autoland/rev/69fcb7b0e100 stylo: Add bindings for filling nsStyleImageLayers lists; r=heycam
Comment 9•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/69fcb7b0e100
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Updated•8 years ago
|
Blocks: stylo-nightly
You need to log in
before you can comment on or make changes to this bug.
Description
•