Closed
Bug 1180107
Opened 9 years ago
Closed 8 years ago
Refactor out helper-function "CanMainSizeInfluenceCrossSize" for use in flex layout
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla49
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(1 file)
5.33 KB,
patch
|
MatsPalmgren_bugz
:
review+
lizzard
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Filing this bug for some patches that continue the flexbox logical-dimensions conversion work done in bug 115829.
Assignee | ||
Comment 1•9 years ago
|
||
This patch doesn't affect behavior at all -- it just refactors an existing multi-part condition into a helper-function. (and makes the logic easier to follow by considering the cases separately) This is a step towards removing a IsCrossAxisHorizontal() call (in the condition that I'm refactoring out here). That removal will happen in a later patch here.
Attachment #8629234 -
Flags: review?(mats)
Comment 2•9 years ago
|
||
Comment on attachment 8629234 [details] [diff] [review] fix: Refactor out FlexItem::CanMainSizeInfluenceCrossSize >+ // Once the main size has been resolved, should we bother doing layout to >+ // establish the cross size? >+ bool CanMainSizeInfluenceCrossSize(const FlexboxAxisTracker& aAxisTracker); ... >+// Indicates whether it's possible for main-size resolution to influence this >+// item's cross size. >+bool >+FlexItem::CanMainSizeInfluenceCrossSize(const FlexboxAxisTracker& aAxisTracker) The latter comment seems redundant. It looks like the method can be 'const'?
Attachment #8629234 -
Flags: review?(mats) → review+
Assignee | ||
Comment 3•8 years ago
|
||
Sorry for letting this bug go quiet! There's more work to be done here, but I'd like to go ahead & land the attached patch, independent of the remaining logical-coordinates work. So: I'm morphing this bug to be focused on the attached patch, and I spun off bug 1267462 for the remaining logical-coordinate conversions.
Summary: Use logical dimensions/coordinates more in flex layout → Refactor out helper-function "CanMainSizeInfluenceCrossSize" for use in flex layout
Assignee | ||
Updated•8 years ago
|
Attachment #8629234 -
Attachment description: part 1: Refactor out FlexItem::CanMainSizeInfluenceCrossSize → fix: Refactor out FlexItem::CanMainSizeInfluenceCrossSize
Assignee | ||
Comment 5•8 years ago
|
||
Addressed review comments & landed. ^^
status-firefox42:
affected → ---
Flags: in-testsuite-
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5e212cfe5849
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Updated•8 years ago
|
Target Milestone: mozilla48 → mozilla49
Assignee | ||
Comment 7•8 years ago
|
||
Comment on attachment 8629234 [details] [diff] [review] fix: Refactor out FlexItem::CanMainSizeInfluenceCrossSize Requesting approval to uplift this refactoring-patch. In fact, I really want to uplift the patches in bug 1030952 -- and that bug's patches layer on top of this patch, so we need this patch first. (bug 1030952 modifies the helper-function that I'm refactoring out in this patch.) Approval Request Comment [Feature/regressing bug #]: N/A [User impact if declined]: Won't be able to backport bug 1030952 (which will mean images & other elements with an aspect ratio may get stretched only in Firefox) [Describe test coverage new/current, TreeHerder]: Our flexbox code has good test coverage (even better after the tests added in bug 1030952). This patch has been getting testing on Nightly for a few days now. [Risks and why]: This is as low-risk as it gets. This patch is just refactoring out an existing check into a helper-function, so that it's easier to understand & modify later on. No functional changes. [String/UUID change made/needed]: None.
Attachment #8629234 -
Flags: approval-mozilla-aurora?
Comment 8•8 years ago
|
||
Comment on attachment 8629234 [details] [diff] [review] fix: Refactor out FlexItem::CanMainSizeInfluenceCrossSize Has coverage on m-c, plus we want dholbert's flexbox refactoring world domination plans to happen
Attachment #8629234 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 9•8 years ago
|
||
Thanks! :D
Comment 10•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/26100e0d8a4d
status-firefox48:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•