Closed
Bug 1418899
Opened 7 years ago
Closed 7 years ago
move some static methods out of nsRuleNode
Categories
(Core :: CSS Parsing and Computation, enhancement)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: heycam, Assigned: heycam)
References
Details
Attachments
(4 files, 1 obsolete file)
We need to keep calling these methods after removing the old style system.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8929989 -
Flags: review?(cku)
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8929987 [details] Bug 1418899 - Part 2: Move some font-related static methods out of nsRuleNode. https://reviewboard.mozilla.org/r/201170/#review206266 C/C++ static analysis found 1 defect in this patch. You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp` ::: layout/base/nsLayoutUtils.cpp:10167 (Diff revision 1) > + float devPerCSS = > + (float)nsPresContext::AppUnitsPerCSSPixel() / > + aPresContext->DeviceContext()->AppUnitsPerDevPixelAtUnitFullZoom(); > + nsAutoString systemFontName; > + if (LookAndFeel::GetFont(aFontID, systemFontName, fontStyle, devPerCSS)) { > + systemFontName.Trim("\"'"); Warning: Escaped string literal can be written as a raw string literal [clang-tidy: modernize-raw-string-literal] systemFontName.Trim("\"'"); ^ R"("')"
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8929986 [details] Bug 1418899 - Part 1: Remove some unused method declarations on nsRuleNode. https://reviewboard.mozilla.org/r/201168/#review206280
Attachment #8929986 -
Flags: review?(tlin) → review+
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8929987 [details] Bug 1418899 - Part 2: Move some font-related static methods out of nsRuleNode. https://reviewboard.mozilla.org/r/201170/#review206284 ::: layout/base/nsLayoutUtils.h:3058 (Diff revision 1) > > // Return the default value to be used for -moz-control-character-visibility, > // from preferences. > static uint8_t ControlCharVisibilityDefault(); > > + enum class FlushUserFontSet { Nit: While we're here, perhaps make this uint8_t to save some memory?
Attachment #8929987 -
Flags: review?(tlin) → review+
Assignee | ||
Comment 9•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8929987 [details] Bug 1418899 - Part 2: Move some font-related static methods out of nsRuleNode. https://reviewboard.mozilla.org/r/201170/#review206284 > Nit: While we're here, perhaps make this uint8_t to save some memory? It probably doesn't make much difference, since we only use this as an argument value, so we'll be pushing a whole word on the stack anyway.
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8929988 [details] Bug 1418899 - Part 3: Use a single image layer array filling function. https://reviewboard.mozilla.org/r/201172/#review206290
Attachment #8929988 -
Flags: review?(cku) → review+
Comment 11•7 years ago
|
||
mozreview-review |
Comment on attachment 8929990 [details] Bug 1418899 - Part 4: Move image layer filling function out of nsRuleNode. https://reviewboard.mozilla.org/r/201176/#review206292
Attachment #8929990 -
Flags: review?(cku) → review+
Attachment #8929989 -
Flags: review?(cku) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8929989 -
Attachment is obsolete: true
Comment 16•7 years ago
|
||
mozreview-review |
Comment on attachment 8929987 [details] Bug 1418899 - Part 2: Move some font-related static methods out of nsRuleNode. https://reviewboard.mozilla.org/r/201170/#review206666 C/C++ static analysis found 1 defect in this patch. You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp` ::: layout/base/nsLayoutUtils.cpp:10167 (Diff revision 2) > + float devPerCSS = > + (float)nsPresContext::AppUnitsPerCSSPixel() / > + aPresContext->DeviceContext()->AppUnitsPerDevPixelAtUnitFullZoom(); > + nsAutoString systemFontName; > + if (LookAndFeel::GetFont(aFontID, systemFontName, fontStyle, devPerCSS)) { > + systemFontName.Trim("\"'"); Warning: Escaped string literal can be written as a raw string literal [clang-tidy: modernize-raw-string-literal] systemFontName.Trim("\"'"); ^ R"("')"
Comment 17•7 years ago
|
||
Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/59639cc071fd Part 1: Remove some unused method declarations on nsRuleNode. r=TYLin https://hg.mozilla.org/integration/autoland/rev/58773b3c8b85 Part 2: Move some font-related static methods out of nsRuleNode. r=TYLin https://hg.mozilla.org/integration/autoland/rev/2d6409b7bbfd Part 3: Use a single image layer array filling function. r=cjku https://hg.mozilla.org/integration/autoland/rev/3f7025b5d57e Part 4: Move image layer filling function out of nsRuleNode. r=cjku
Comment 18•7 years ago
|
||
Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/71b0dd5f14d3 Followup speculative Windows build fix. (CLOSED TREE)
Comment 19•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/59639cc071fd https://hg.mozilla.org/mozilla-central/rev/58773b3c8b85 https://hg.mozilla.org/mozilla-central/rev/2d6409b7bbfd https://hg.mozilla.org/mozilla-central/rev/3f7025b5d57e https://hg.mozilla.org/mozilla-central/rev/71b0dd5f14d3
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in
before you can comment on or make changes to this bug.
Description
•