Closed Bug 1383296 Opened 4 years ago Closed 4 years ago

Remove the nsCSSValue::Serialization enum and remove decision logic in functions that use it

Categories

(Core :: CSS Parsing and Computation, enhancement)

Product:
Core
Component:
CSS Parsing and Computation
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: bradwerth, Assigned: bradwerth)

References

Details

Attachments

(1 file)

Bug 1383296 Part 1: Remove all uses of the nsCSSValue::Serialization enum, now that it no longer has multiple values.
59 bytes, text/x-review-board-request
heycam
: review+
Details 
If eAuthorSpecified is removed in Bug 1302513, then the only remaining value for nsCSSValue::Serialization will be eNormalized. Unless we want to maintain this enum for future serialization options, we can remove this parameter in all functions that use it, and the corresponding logic.
Summary: Remove the nsCSSValue::Serialization enum → Remove the nsCSSValue::Serialization enum and remove decision logic in functions that use it
Any reason to keep this code with only one value in the enum?
Flags: needinfo?(dholbert)
Superficial answer: doesn't seem like it.

(For a non-superficial answer, I'd suggest looking up the history of the enum using "hg blame" on e.g. searchfox.org, to see when/why it was added, to verify that its reasons to exist [back then] will now be gone.  It's possible that might might turn up additional simplifications that could be made at the same time, too.)
Flags: needinfo?(dholbert)
Cameron, fully removing serialization enum would undo much of the work in Bug 731271. Do you feel that we should retain the serialization parameter (now with only one value in the enum)?
Flags: needinfo?(cam)
Assignee: nobody → bwerth
Attachment #8888999 - Flags: review?(cam)
Removing the enum sounds like the right thing to do, thank you!
Flags: needinfo?(cam)
Comment on attachment 8888999 [details]
Bug 1383296 Part 1: Remove all uses of the nsCSSValue::Serialization enum, now that it no longer has multiple values.

https://reviewboard.mozilla.org/r/160034/#review165564

Thanks for cleaning this up!

::: layout/style/nsCSSValue.h:995
(Diff revision 1)
>    void AppendBasicShapePositionToString(
> -           nsAString& aResult,
> +           nsAString& aResult) const;

Nit: this could probably fit on one line now.
Attachment #8888999 - Flags: review?(cam) → review+
Pushed by bwerth@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/acf2dcc2c76a
Part 1: Remove all uses of the nsCSSValue::Serialization enum, now that it no longer has multiple values. r=heycam
https://hg.mozilla.org/mozilla-central/rev/acf2dcc2c76a
Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.