Page MenuHomePhabricator

Bug 1554433 - Move system colors to values::specified::color. r=#style,stransky
ClosedPublic

Authored by emilio on May 26 2019, 2:45 AM.

Details

Summary

This should be an idempotent patch. The way to come up with this patch has been:

  • Run the first script attached to the bug and pipe it to xclip, then paste it in color.rs
  • Add the relevant #[derive] annotations and remove the color.mako.rs definition.
  • Reorder the values to match the ColorID definition, on which some widget prefs and caching stuff relies on.
  • Manually port some documentation from nsLookAndFeel.h
  • Run rg 'eColorID_' | cut -d : -f 1 | sort | uniq >files
  • Run the second script attached to the bug.
  • Manually fix usage of LAST_COLOR (adding the End variant), and adding casts to integer as needed.
  • Add an static assert so that people remember to update the prefs, rather than a comment on the definition :)

Diff Detail

Repository
rMOZILLACENTRAL mozilla-central
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

emilio created this revision.May 26 2019, 2:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 26 2019, 2:45 AM
phab-bot requested review of this revision.May 26 2019, 2:46 AM
phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".
phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".
phab-bot removed a project: Restricted Project.

Code analysis found 3 defects in the diff 110049:

  • 2 defects found by clang-format
  • 1 defect found by clang-tidy

You can run this analysis locally with:

  • ./mach clang-format -s -p widget/nsXPLookAndFeel.h (C/C++)
  • ./mach static-analysis check widget/nsXPLookAndFeel.cpp (C/C++)

For your convenience, here is a patch that fixes all the clang-format defects (use it in your repository with hg import or git apply).

If you see a problem in this automated review, please report it here.

widget/nsXPLookAndFeel.cpp
962

Warning: Statement should be inside braces [clang-tidy: readability-braces-around-statements]
Checker reliability is high (false positive risk).

xidorn accepted this revision as: xidorn.May 26 2019, 10:29 AM
xidorn added a subscriber: xidorn.
xidorn added inline comments.
widget/nsXPLookAndFeel.cpp
97

This comment needs update.

This revision is now accepted and ready to land.May 26 2019, 10:29 AM
xidorn removed a reviewer: Restricted Project.May 26 2019, 10:29 AM
emilio updated this revision to Diff 110093.May 26 2019, 1:06 PM

Fix android build and use simpler casts

emilio edited the summary of this revision. (Show Details)May 26 2019, 1:06 PM
emilio marked an inline comment as done.May 26 2019, 1:10 PM
This revision was automatically updated to reflect the committed changes.

Code analysis found 1 defect in the diff 110093:

  • 1 defect found by clang-tidy

You can run this analysis locally with:

  • ./mach static-analysis check widget/nsXPLookAndFeel.cpp (C/C++)

If you see a problem in this automated review, please report it here.

widget/nsXPLookAndFeel.cpp
962

Warning: Statement should be inside braces [clang-tidy: readability-braces-around-statements]
Checker reliability is high (false positive risk).