Closed
Bug 1448763
Opened 6 years ago
Closed 6 years ago
Remove nsCSSValue::AppendToString and some other unused stuff in nsCSSValue
Categories
(Core :: CSS Parsing and Computation, enhancement)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: xidorn, Assigned: xidorn)
References
Details
Attachments
(10 files)
59 bytes,
text/x-review-board-request
|
emilio
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
emilio
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
emilio
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
emilio
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
emilio
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
emilio
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
emilio
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
emilio
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
emilio
:
review+
|
Details |
41 bytes,
text/x-github-pull-request
|
Details | Review |
That is quite a bit of unused code now. The only remaining uses as far as I can see are serialization of descriptors of @counter-style and @font-face. We should replace them with Servo serialization or just move the needed code to those files directly rather than keeping the huge serialization code.
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → xidorn+moz
Assignee | ||
Comment 1•6 years ago
|
||
So there are two remaining references to the serialization code: * nsComputedDOMStyle::DoGetAlignContent uses nsCSSValue::AppendAlignJustifyValueToString * DOMIntersectionObserver::GetRootMargin uses nsCSSRect::AppendToString
Assignee | ||
Updated•6 years ago
|
Summary: Remove nsCSSValue::AppendToString and its friends → Remove nsCSSValue::AppendToString and other unused stuff in nsCSSValue
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 11•6 years ago
|
||
I believe majority of units and methods of nsCSSValue can be removed nowadays, but that need some more time to investigate. I guess it probably makes more sense to remove the whole nsCSSValue at some point when we can drop all dependencies to it.
Assignee | ||
Updated•6 years ago
|
Summary: Remove nsCSSValue::AppendToString and other unused stuff in nsCSSValue → Remove nsCSSValue::AppendToString and some other unused stuff in nsCSSValue
Comment 12•6 years ago
|
||
mozreview-review |
Comment on attachment 8965211 [details] Bug 1448763 part 1 - Make DOMIntersectionObserver use nsStyleSides for mRootMargin, and use Servo code to serialize it. https://reviewboard.mozilla.org/r/233912/#review239554 ::: dom/base/DOMIntersectionObserver.cpp:125 (Diff revision 1) > } > > void > DOMIntersectionObserver::GetRootMargin(mozilla::dom::DOMString& aRetVal) > { > - mRootMargin.AppendToString(eCSSProperty_DOM, aRetVal); > + nsString& retVal = aRetVal; Huh, is this needed? I'd expect &aRetVal to just work.
Attachment #8965211 -
Flags: review?(emilio) → review+
Comment 13•6 years ago
|
||
mozreview-review |
Comment on attachment 8965212 [details] Bug 1448763 part 2 - Remove serialization code for specified value. https://reviewboard.mozilla.org/r/233914/#review239556 Nice!
Attachment #8965212 -
Flags: review?(emilio) → review+
Comment 14•6 years ago
|
||
mozreview-review |
Comment on attachment 8965213 [details] Bug 1448763 part 3 - Remove nsCSSValueTokenStream. https://reviewboard.mozilla.org/r/233916/#review239558
Attachment #8965213 -
Flags: review?(emilio) → review+
Comment 15•6 years ago
|
||
mozreview-review |
Comment on attachment 8965214 [details] Bug 1448763 part 4 - Remove nsCSSValueGradient and its friends. https://reviewboard.mozilla.org/r/233918/#review239560
Attachment #8965214 -
Flags: review?(emilio) → review+
Comment 16•6 years ago
|
||
mozreview-review |
Comment on attachment 8965215 [details] Bug 1448763 part 5 - Remove all color stuff from nsCSSValue. https://reviewboard.mozilla.org/r/233920/#review239562
Attachment #8965215 -
Flags: review?(emilio) → review+
Comment 17•6 years ago
|
||
mozreview-review |
Comment on attachment 8965216 [details] Bug 1448763 part 6 - Remove nsCSSValueTriplet. https://reviewboard.mozilla.org/r/233922/#review239564
Attachment #8965216 -
Flags: review?(emilio) → review+
Comment 18•6 years ago
|
||
mozreview-review |
Comment on attachment 8965217 [details] Bug 1448763 part 7 - Remove nsCSSCornerSizes. https://reviewboard.mozilla.org/r/233924/#review239566
Attachment #8965217 -
Flags: review?(emilio) → review+
Comment 19•6 years ago
|
||
mozreview-review |
Comment on attachment 8965218 [details] Bug 1448763 part 8 - Remove CSSCalc.h. https://reviewboard.mozilla.org/r/233926/#review239568
Attachment #8965218 -
Flags: review?(emilio) → review+
Comment 20•6 years ago
|
||
mozreview-review |
Comment on attachment 8965219 [details] Bug 1448763 part 9 - Remove nsCSSRect. https://reviewboard.mozilla.org/r/233928/#review239570
Attachment #8965219 -
Flags: review?(emilio) → review+
Assignee | ||
Comment 21•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8965211 [details] Bug 1448763 part 1 - Make DOMIntersectionObserver use nsStyleSides for mRootMargin, and use Servo code to serialize it. https://reviewboard.mozilla.org/r/233912/#review239554 > Huh, is this needed? I'd expect &aRetVal to just work. Yes, it is needed, it cannot automatically convert a `DOMString*` to `nsAString*`, we need a semi-explicit conversion.
Assignee | ||
Comment 22•6 years ago
|
||
Comment 23•6 years ago
|
||
Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d746dd4fe0e8 part 1 - Make DOMIntersectionObserver use nsStyleSides for mRootMargin, and use Servo code to serialize it. r=emilio https://hg.mozilla.org/integration/autoland/rev/ce6f0d98da1f part 2 - Remove serialization code for specified value. r=emilio https://hg.mozilla.org/integration/autoland/rev/64aab4d047bd part 3 - Remove nsCSSValueTokenStream. r=emilio https://hg.mozilla.org/integration/autoland/rev/9c43f7ce61e1 part 4 - Remove nsCSSValueGradient and its friends. r=emilio https://hg.mozilla.org/integration/autoland/rev/1754cae1e273 part 5 - Remove all color stuff from nsCSSValue. r=emilio https://hg.mozilla.org/integration/autoland/rev/e56285e2657c part 6 - Remove nsCSSValueTriplet. r=emilio https://hg.mozilla.org/integration/autoland/rev/f43518ccb6f9 part 7 - Remove nsCSSCornerSizes. r=emilio https://hg.mozilla.org/integration/autoland/rev/c44d6b666b2f part 8 - Remove CSSCalc.h. r=emilio https://hg.mozilla.org/integration/autoland/rev/7c0093bf53b5 part 9 - Remove nsCSSRect. r=emilio
Comment 24•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d746dd4fe0e8 https://hg.mozilla.org/mozilla-central/rev/ce6f0d98da1f https://hg.mozilla.org/mozilla-central/rev/64aab4d047bd https://hg.mozilla.org/mozilla-central/rev/9c43f7ce61e1 https://hg.mozilla.org/mozilla-central/rev/1754cae1e273 https://hg.mozilla.org/mozilla-central/rev/e56285e2657c https://hg.mozilla.org/mozilla-central/rev/f43518ccb6f9 https://hg.mozilla.org/mozilla-central/rev/c44d6b666b2f https://hg.mozilla.org/mozilla-central/rev/7c0093bf53b5
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in
before you can comment on or make changes to this bug.
Description
•