Scrollframe question

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Scrollframe question

Boris Zbarsky
I'm looking at the mSupppressScrollbarUpdate of scrollframe.  It's only used by
nsListControlFrame, which calls SetSuppressScrollbarUpdate() when it does its
first-pass reflow.  It seems to think that this prevents scrolling to the
restored spot...

Is that actually the case?  I can't quite figure out where (or whether!) the
code conditioned on mSupppressScrollbarUpdate actually does any scrolling.

-Boris
_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout
Reply | Threaded
Open this post in threaded view
|

Re: Scrollframe question

Robert O'Callahan-3
Boris Zbarsky wrote:
> I'm looking at the mSupppressScrollbarUpdate of scrollframe.  It's only
> used by nsListControlFrame, which calls SetSuppressScrollbarUpdate()
> when it does its first-pass reflow.  It seems to think that this
> prevents scrolling to the restored spot...

Well that's definitely wrong...

> Is that actually the case?  I can't quite figure out where (or whether!)
> the code conditioned on mSupppressScrollbarUpdate actually does any
> scrolling.

It can. nsGfxScrollFrameInner::LayoutScrollbars sets the scrollbar
maxpos, which (in nsSliderFrame::AttributeChanged) is where we check if
the current position is now out-of-bounds, and if so, scroll it to be
in-bounds. So I suspect the listbox code is trying to avoid a situation
where the listbox is scrolled to the bottom, and the first-pass reflow
gives a temporarily-shorter height than the true height, which would
cause the listbox to spuriously scroll up.

Rob
_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout
Reply | Threaded
Open this post in threaded view
|

Re: Scrollframe question

Boris Zbarsky
Robert O'Callahan wrote:
> So I suspect the listbox code is trying to avoid a situation
> where the listbox is scrolled to the bottom, and the first-pass reflow
> gives a temporarily-shorter height than the true height, which would
> cause the listbox to spuriously scroll up.

OK.  I think I'm actually going to ignore this eventuality, with the following
comment:

   /*
    * I don't think we want to SetSuppressScrollbarUpdate() here.  The only case
    * we'd _want_ to suppress it is the case when we're going to do both reflow
    * passes and the first pass will produce a height that is both smaller than
    * the current height and smaller than the height we'll eventually end up
    * with, since in that situation we'd end up clamping the scroll position to
    * a too-small value after the first reflow.  The only way that can happen is
    * if our size attr has decreased (so that we'll end up with a smaller height
    * than our current height), and at the same time the size of our tallest
    * option has increased.  This is a pretty rare case, and not calling
    * SetSuppressScrollbarUpdate() means we can get away without a second reflow
    * in other (much more common) cases.
    */

If you buy this, we can remove SetSuppressScrollbarUpdate altogether, I think.  ;)

-Boris
_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout
Reply | Threaded
Open this post in threaded view
|

Re: Scrollframe question

Robert O'Callahan-3
Actually I had it backwards; the problem is when the height after the
first pass is too *large*, then we set the scrollbar maxpos to zero and
scroll to the top of the listbox. See
https://bugzilla.mozilla.org/show_bug.cgi?id=301439#c8. If you can avoid
regressing that testcase, then I'm happy :-).

Rob
_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout
Reply | Threaded
Open this post in threaded view
|

Re: Scrollframe question

Boris Zbarsky
Robert O'Callahan wrote:
> Actually I had it backwards; the problem is when the height after the
> first pass is too *large*, then we set the scrollbar maxpos to zero and
> scroll to the top of the listbox. See
> https://bugzilla.mozilla.org/show_bug.cgi?id=301439#c8. If you can avoid
> regressing that testcase, then I'm happy :-).

Well.  That testcase won't regress because it only does one-pass reflow in the
new world.  A testcase that _would_ regress is one in which the font-size is
decreased at the same time as the "size" attribute is increased.  Then we'd do a
first-pass reflow using the old font size and the new "size" attribute, figure
out that the size of our options has changed, and do a second reflow at the
right (smaller) size.

Unfortunately, with the new setup we don't know until we're done with the first
reflow whether we'll have to do a second one...  I suppose I could always do two
reflows, and the second one would always have to do _some_ work because it'd
need to update the scrollbars.  But I'd really like to avoid that if I can; it
seems wasteful.

-Boris
_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout
Reply | Threaded
Open this post in threaded view
|

Re: Scrollframe question

Robert O'Callahan-3
Boris Zbarsky wrote:

> Robert O'Callahan wrote:
>> Actually I had it backwards; the problem is when the height after the
>> first pass is too *large*, then we set the scrollbar maxpos to zero and
>> scroll to the top of the listbox. See
>> https://bugzilla.mozilla.org/show_bug.cgi?id=301439#c8. If you can avoid
>> regressing that testcase, then I'm happy :-).
>
> Well.  That testcase won't regress because it only does one-pass reflow
> in the new world.  A testcase that _would_ regress is one in which the
> font-size is decreased at the same time as the "size" attribute is
> increased.  Then we'd do a first-pass reflow using the old font size and
> the new "size" attribute, figure out that the size of our options has
> changed, and do a second reflow at the right (smaller) size.
>
> Unfortunately, with the new setup we don't know until we're done with
> the first reflow whether we'll have to do a second one...  I suppose I
> could always do two reflows, and the second one would always have to do
> _some_ work because it'd need to update the scrollbars.  But I'd really
> like to avoid that if I can; it seems wasteful.

Yeah, definitely.

Maybe you could have the nsSelectsAreaFrame detect whether a second
reflow will be needed, before it returns from its reflow near the end of
the first pass, and set up suppression from there if necessary.

Rob
_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout
Reply | Threaded
Open this post in threaded view
|

Re: Scrollframe question

Boris Zbarsky
Robert O'Callahan wrote:
> Maybe you could have the nsSelectsAreaFrame detect whether a second
> reflow will be needed, before it returns from its reflow near the end of
> the first pass, and set up suppression from there if necessary.

Hmm...  That might work.  I'll poke at it.

Thanks,
-Boris
_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout