Closed
Bug 732610
Opened 12 years ago
Closed 12 years ago
Make nsIFrame::ComputeSize take a bitfield |aFlags| instead of |bool aShrinkWrap|
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(1 file)
31.74 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
At the beginning of the flexbox algorithm, we need to get the computed size (possibly the shrinkwrapped size) of each child-frames, *without* taking min/max width/height into consideration.[1] We can't easily do this right now, though. nsIFrame::ComputeSize is the obvious method to use, but it applies min/max constraints to the returned values. THEREFORE: I'd like to allow nsIFrame::ComputeSize to optionally ignore min/max. To do that, I'd like to replace the final |bool aShrinkWrap| ComputeSize argument with a "flags" bitfield (with eShrinkWrap being the first flag in that field). [1] The flexbox algorithm starts with these sizes, _then_ assigns extra space based on flexibility, and _then_ clamps based on min/max constraints. If we apply min/max constraints too early, it'll produce different, incorrect behavior, since there will be a different amount of available space.
Assignee | ||
Comment 1•12 years ago
|
||
Not sure this is complete/correct, but it compiles. Will request review after a bit more sanity-checking.
Assignee | ||
Updated•12 years ago
|
Attachment #602516 -
Flags: review?(dbaron)
Assignee | ||
Updated•12 years ago
|
Whiteboard: [autoland:-p linux -u reftest]
Assignee | ||
Updated•12 years ago
|
Whiteboard: [autoland:-p linux -u reftest] → [autoland-try:-p linux -u reftest]
Updated•12 years ago
|
Whiteboard: [autoland-try:-p linux -u reftest] → [autoland-in-queue]
Comment 2•12 years ago
|
||
Autoland Patchset: Patches: 602516 Branch: mozilla-central => try Destination: http://hg.mozilla.org/try/pushloghtml?changeset=65c185a8ba68 Try run started, revision 65c185a8ba68. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=65c185a8ba68
Comment 3•12 years ago
|
||
Try run for 65c185a8ba68 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=65c185a8ba68 Results (out of 6 total builds): success: 6 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-65c185a8ba68
Updated•12 years ago
|
Whiteboard: [autoland-in-queue]
Comment on attachment 602516 [details] [diff] [review] fix v1 r=dbaron I wonder if you should make the same conversion apply to ComputeAutoSize, though. We should definitely use MOZ_OVERRIDE more aggressively in layout.
Attachment #602516 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 5•12 years ago
|
||
(In reply to David Baron [:dbaron] from comment #4) > I wonder if you should make the same conversion apply to ComputeAutoSize, > though. Possibly, yeah. I didn't do that yet because the one flag that I'm interested in adding (for "ignore min/max width/height properties") isn't needed in ComputeAutoSize. (That method already ignores min/max constraints.) It might be good for consistency & future-proofing, though. Let me know if you'd like me to do that as part of this bug; otherwise I'll hold off on it for now. > We should definitely use MOZ_OVERRIDE more aggressively in layout. Agreed. I've just filed bug 733186 on that.
Assignee | ||
Comment 6•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/cfd17c20d8e9
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/cfd17c20d8e9
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•