Expose xul:scrollbar attributes as XBL properties

NEW
Unassigned

Status

()

Core
XUL
--
enhancement
14 years ago
10 years ago

People

(Reporter: WeirdAl, Unassigned)

Tracking

Trunk
x86
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 obsolete attachments)

(Reporter)

Description

14 years ago
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.
(Reporter)

Comment 1

14 years ago
Created attachment 148060 [details] [diff] [review]
patch, v1
Assignee: nobody → ajvincent
Status: NEW → ASSIGNED
(Reporter)

Comment 2

14 years ago
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?
Attachment #148060 - Flags: review?(neil.parkwaycc.co.uk)
(Reporter)

Comment 3

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

Comment 4

14 years ago
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.
Attachment #148060 - Flags: review?(neil.parkwaycc.co.uk) → review-

Comment 5

14 years ago
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.
(Reporter)

Comment 6

14 years ago
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.
Aren't we trying to move away from storing this stuff as attributes?

Also, are you sure you want curpos, say, in twips?
(Reporter)

Comment 8

14 years ago
(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?

(Reporter)

Comment 9

14 years ago
Created attachment 148130 [details] [diff] [review]
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.)
Attachment #148060 - Attachment is obsolete: true
> 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.
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.)
> nsGfxScrollFrame::SetAttribute converts its parameter from twips to pixels

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

Comment 13

14 years ago
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.
Attachment #148130 - Flags: review?(neil.parkwaycc.co.uk)

Comment 14

14 years ago
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.
(Reporter)

Comment 15

14 years ago
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

14 years ago
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.
(Reporter)

Comment 17

14 years ago
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.
Attachment #148130 - Attachment is obsolete: true
Attachment #148130 - Flags: review?(neil.parkwaycc.co.uk)
(Reporter)

Updated

12 years ago
Assignee: ajvincent → nobody
Status: ASSIGNED → NEW
QA Contact: xptoolkit.xul

Updated

10 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.