Closed
Bug 1321056
Opened 8 years ago
Closed 8 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•8 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•8 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•8 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•8 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•8 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•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in
before you can comment on or make changes to this bug.
Description
•