243128 - Expose xul:scrollbar attributes as XBL properties

Status: RESOLVED INVALID

(Core :: XUL, enhancement)

Description

[Alex Vincent [:WeirdAl]]

The features of XUL scrollbars are a little hard to script.  I have a patch 
which takes these features and exposes them as properties which scripts can 
access directly.

Comment 1

[Alex Vincent [:WeirdAl]]

Attachment: patch, v1

Comment 2

[Alex Vincent [:WeirdAl]]

Comment on attachment 148060 [details] [diff] [review]
patch, v1

*sigh* Thank you very much, Patch Maker, for specifying the wrong file.

toolkit/content/global/bindings/scrollbar.xml
should be
xpfe/global/resources/content/bindings/scrollbar.xml

timeless: who do you think should review this, if not neil?

Comment 3

[Alex Vincent [:WeirdAl]]

(if this patch is approved, I'd like opinions on whether a percentage-based set 
of properties is appropriate as well).

Comment 4

[[e-mail]]

Comment on attachment 148060 [details] [diff] [review]
patch, v1

r=me on the properties, but not the thumb; even if you can convince me you need
it you're using the wrong attribute to get it.

Comment 5

[[e-mail]]

Trying to guess why you want the thumb from reading your blog I'll just point
out that since fairly recently scrollbars proportion themselves based on their
maxpos and pageIncrement attributes (older scrollbars used their size instead).

Also, for the curpos, maxpos, increment and pageIncrement attributes you might
want to convert them to integers, defaulting to 0, 100, 1 and 10 respectively.

Comment 6

[Alex Vincent [:WeirdAl]]

Fast reviews are always the best, even if they sting a little.  Expect a new 
patch tomorrow to conform to specifications (and get the right filename).  I'll 
drop the thumb property and the px bits.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Aren't we trying to move away from storing this stuff as attributes?

Also, are you sure you want curpos, say, in twips?

Comment 8

[Alex Vincent [:WeirdAl]]

(In reply to comment #7)
> Aren't we trying to move away from storing this stuff as attributes?

That would be nice.  The only problem is that the current setup is attribute-
based, not property-based.  

> Also, are you sure you want curpos, say, in twips?

What's twips?

Comment 9

[Alex Vincent [:WeirdAl]]

Attachment: patch, v1.1

I didn't set the properties as integers in this case; the assumption is that
the user may end up specifying "px", %, or some other figure.  (Testing shows
the current implementation of xul:scrollbar assumes pixels, and refuses to
consider other units.)

Comment 10

[Boris Zbarsky [:bzbarsky]]

> That would be nice. 

You sure?  The idea would be that the curpos would be stored somewhere in
layout, not in the scrollbar content node.

> What's twips?

The units that curpos is measured in.  20 twips == 1 pt.

> (Testing shows the current implementation of xul:scrollbar assumes pixels

I find that very hard to believe, since all layout consumers of curpos assume
it's in twips.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Actually Boris, curpos and the other attributes are in pixels.
nsGfxScrollFrame::SetAttribute converts its parameter from twips to pixels, if
that was confusing you.

It seems to me that Alex's patch will actually ease the transition to
internalizing the scrollbar properties. Javascript users can be transitioned to
use these XBL properties and we'll change the property implementation to call
back C++ methods on the scrollbar. (How should that work, anyway? I'm all hazy
about content. I assume I want to do whatever the DOM does.)

Comment 12

[Boris Zbarsky [:bzbarsky]]

> nsGfxScrollFrame::SetAttribute converts its parameter from twips to pixels

Ah, ok.  Yeah, that's the part I missed.

Comment 13

[Alex Vincent [:WeirdAl]]

Comment on attachment 148130 [details] [diff] [review]
patch, v1.1

roc: re comment 11, I don't have a clue how the DOM content model interacts
with it.  Though in the blog entry the URL field references, there is a link to
a testcase you can stick into DOM Inspector.  (It didn't help me; maybe you can
find it...)

If I may venture an opinion, assuming the scrollbar attributes are in pixels is
evil.  It'd be nice to have bugs on file for (a) changing the property impl. as
roc suggests, and (b) making the scrollbars support other units of measurement.

Comment 14

[[e-mail]]

Al, you're not going to convert them to return integers with defaults? This
would match any internalized functions that we would want to expose.

bz, roc, scrollbars have always been able to use arbitrary units but a) they
don't drag smoothly for units that are larger than pixels b) before I fixed bug
236545 the proportional thumb code assumed pixels.

Comment 15

[Alex Vincent [:WeirdAl]]

Oh, I'm sorry; I thought you meant my example needed to be changed to integers.

Question, then: if the attribute is set to some other system of units (say, 
percentages), how should I adjust the binding to reflect that?

Comment 16

[[e-mail]]

Sorry for the delay but I didn't see your question... I'm still confused by this
units business, if you scale all the values of these attributes it should have
no effect on the appearance of the scrollbar (except that you'll be able to
scroll to intermediate points, of course...), any interpretation of the
attributes into specific units is performed by the owner of the scrollbar, and
iirc all our current owners happen to be written as if the units were twips.

Comment 17

[Alex Vincent [:WeirdAl]]

Comment on attachment 148130 [details] [diff] [review]
patch, v1.1

For the moment I've lost interest in this bug... if anyone wants to take it and
write up a proper patch, be my guest.

Comment 18

[Alex Vincent [:WeirdAl]]

XBL is dead. It has ceased to be.