Closed
Bug 1321056
Opened 7 years ago
Closed 7 years ago
Preemptively fix unified bustage in layout/svg
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla53
| Tracking | Status | |
|---|---|---|
| firefox53 | --- | fixed |
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(1 file)
When reviewing the patches in bug 1304598, I was inspired to preemptively fix unified bustage in a few places, so that changes like bug 1304598 will be easier in the future. We can reveal latent-unified-bustage by simply tweaking moz.build files to replace UNIFIED_SOURCES with SOURCES. I did that in layout/svg and fixed the resulting bustage; patch coming up.
|
Assignee | |
Comment 1•7 years ago
|
||
TYLin, I'll tag you to review this -- hope that's OK with you!
Assignee: nobody → dholbert
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Comment 3•7 years ago
|
||
* All of the new #includes here are for types of the same name that are directly used in the file that I'm tweaking. We either call functions on a variable of that type (e.g. SVGSVGElement.h in one case, for a call to svgElement->GetViewportSize()), or we return that type by value (e.g. for DrawResult), or we call static methods on that type (e.g. for nsSVGUtils)) * The "using namespace mozilla::image" sprinkled throughout the patch is for DrawResult usages (which live in mozilla::image) * I'm adding explicit an namespace to IntPoint in nsSVGClipPathFrame.h (which is currently accidentally-relying on "using namespace" declarations that get picked up from other .cpp files right now). Let me know if anything else isn't clear here. Thanks!
|
||
Comment 4•7 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #0) > We can reveal latent-unified-bustage by simply tweaking moz.build files to > replace UNIFIED_SOURCES with SOURCES. This is a good idea!
|
|
||
Comment 5•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8815429 [details] Bug 1321056: Add includes/namespaces to fix latent unified bustage in layout/svg. https://reviewboard.mozilla.org/r/96326/#review96658 The explanation in comment 3 is clear to me. r=TYLin ::: layout/svg/nsCSSClipPathInstance.cpp:16 (Diff revision 1) > +#include "nsSVGElement.h" > +#include "nsSVGUtils.h" > +#include "nsSVGViewBox.h" Nit: Put the #includes after "nsRuleNode.h" so that they are in alphabetic order.
Attachment #8815429 -
Flags: review?(tlin) → review+
|
|
Assignee | |
Comment 6•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8815429 [details] Bug 1321056: Add includes/namespaces to fix latent unified bustage in layout/svg. https://reviewboard.mozilla.org/r/96326/#review96658 > Nit: Put the #includes after "nsRuleNode.h" so that they are in alphabetic order. I got the alphabet backwards in my head. :D Fixed -- thanks!
| Comment hidden (mozreview-request) |
Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9953c71eb9ba Add includes/namespaces to fix latent unified bustage in layout/svg. r=TYLin
|
||
Comment 9•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/9953c71eb9ba
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in
before you can comment on or make changes to this bug.
Description
•