Clean up related to scroll frame implementation
Categories
(Core :: Layout: Scrolling and Overflow, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox96 | --- | fixed |
People
(Reporter: TYLin, Assigned: TYLin)
References
Details
Attachments
(6 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
These patches are written when I investigate the scrollbar implementation for bug 1715112. All the patches are refactors, and shouldn't change the behavior.
Assignee | ||
Comment 1•2 years ago
|
||
They shouldn't be scattered in nsHTMLScrollFrame::Reflow(). Also, reorder these
assignment operations according to the member variables' appearance order in
ScrollReflowInput.
I also moved the constructor out of the class definition because it becomes a
large method, and I'm going to add private:
section after the methods section
in a later patch.
Updated•2 years ago
|
Assignee | ||
Comment 2•2 years ago
|
||
We've had some APIs passing ScrollReflowInput by reference while some others
using pointer. Let's unify them by using reference everywhere.
Depends on D132440
Assignee | ||
Comment 3•2 years ago
|
||
It should be sufficient to call SetScrollbarMediatorContent
once in
ScrollReflowInput's constructor, which is created in the beginning of
nsHTMLScrollFrame::Reflow(), instead of calling it repeatedly in multiple
TryLayout() calls.
Depends on D132441
Assignee | ||
Comment 4•2 years ago
|
||
Scrollbar's min and pref sizes won't change during reflow, so we can cache them
in ScrollReflowInput to save some repetitive computation in multiple
ReflowScrolledFrame() and TryLayout() calls.
This is also a preparation for Bug 1715112 because we can use the pref sizes to
compute the scrollbar-gutter size in ScrollReflowInput.
Depends on D132442
Assignee | ||
Comment 5•2 years ago
|
||
Make these variables more distinguishable from oldScrolledAreaBounds
and
newScrolledAreaBounds
.
Depends on D132443
Assignee | ||
Comment 6•2 years ago
|
||
Technically, aContentArea
is not 100% wrong; its the content area of the outer
scroll frame, which contains the content area of the inner scrolled frame, the
padding, and the scrollbars. However, it should really be named
aInsideBorderArea
as the caller names it. Otherwise, it is easy to cause
confusion with the content area of the inner scrolled frame.
Also, rename aOldScrollArea
as well so that we use the term "scroll port"
consistently.
Depends on D132444
Pushed by tlin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fb82680677e1 Part 1 - Move ScrollReflowInput members' initialization into its constructor. r=layout-reviewers,emilio https://hg.mozilla.org/integration/autoland/rev/6046902bfc19 Part 2 - Pass ScrollReflowInput argument by reference. r=emilio https://hg.mozilla.org/integration/autoland/rev/3e3076fbab8d Part 3 - Call SetScrollbarMediatorContent in ScrollReflowInput's constructor. r=emilio https://hg.mozilla.org/integration/autoland/rev/1e32e1c2b2db Part 4 - Cache scrollbar sizes in ScrollReflowInput. r=layout-reviewers,emilio https://hg.mozilla.org/integration/autoland/rev/df87dee15817 Part 5 - Rename variables names storing mScrollPort. r=emilio https://hg.mozilla.org/integration/autoland/rev/3e8f817592dc Part 6 - Rename arguments of ScrollFrameHelper::LayoutScrollbars(). r=emilio
Comment 8•2 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fb82680677e1
https://hg.mozilla.org/mozilla-central/rev/6046902bfc19
https://hg.mozilla.org/mozilla-central/rev/3e3076fbab8d
https://hg.mozilla.org/mozilla-central/rev/1e32e1c2b2db
https://hg.mozilla.org/mozilla-central/rev/df87dee15817
https://hg.mozilla.org/mozilla-central/rev/3e8f817592dc
Description
•