arrowscrollbox - add orient="horizontal"

NEW
Unassigned

Status

()

defect
18 years ago
7 years ago

People

(Reporter: pali, Unassigned)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 obsolete attachments)

Reporter

Description

18 years ago
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.4+) Gecko/20010928
BuildID:    2001092803

Please add orient="horizontal" to arrowscrollbox. I need it to show long toolbars.

Reproducible: Always
Steps to Reproduce:
set orient="horizontal" on <arrowscrollbox> in your XUL

Actual Results:  vertical arrowscrollbox

Expected Results:  horizontal arrowscrollbox

--mondo

Comment 1

18 years ago
http://lxr.mozilla.org/seamonkey/source/xpfe/global/resources/content/bindings/
scrollbox.xml
so that hardwire's the containing box as a vbox. However, I'm not sure 
whether this widget would actually deal with horizontal scrolling. Maybe
there's some intent to do this another way.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

18 years ago
I was able to easily modify the arrowscrollbox binding in
global/resources/content/bindings/scrollbox.xml to enable a horizontal
<arrowscrollbox/> by inheriting the orientation of the bound element.  The
scrollByIndex() method of the nsIScrollBoxObject interface automatically knows
which direction to scroll based on the orientation of the scrollbox (yay!).  So
most of the changes were stylesheet modifications for the arrows.

The default orientation (no orient attr. on the <arrowscrollbox/>) is horizontal
since that's the default orientation for boxes.  This shouldn't be an issue
since the only reference to <arrowscrollbox/> I could find in the tree already
specifies orient="vertical".  This change will affect theme authors.

The patch I'm attaching was made using the patch-maker utility on an Oct.8
build. Please review.
Reporter

Updated

18 years ago
Keywords: patch
Reporter

Comment 4

18 years ago
I've tested the patch and found it to work perfectly. Please do the official
review and check it into tree ASAP.
Removing RFE from summary, because this is valid bug rather then RFE.

--mondo
Summary: RFE: arrowscrollbox - add orient="horizontal" → arrowscrollbox - add orient="horizontal"
Reporter

Updated

18 years ago
Blocks: 70753

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0.1
Reporter

Comment 5

18 years ago
hyatt: no offence, but what are you waiting for? This patch is working great.
What are the reasons for setting target milestone post 1.0? I know, this does
affect theme creators - but the change is really minor and patch includes change
for both supported themes. I really don't see the reason. Thank you for
explaining this to me. Regards
--mondo

Updated

18 years ago
Keywords: mozilla0.9.8

Comment 6

18 years ago
Adding review not sure why this patch is still sitting around.
Keywords: review

Comment 7

18 years ago
doubt this will make 0.9.8
Keywords: mozilla0.9.9

Updated

18 years ago
Blocks: 123569

Comment 8

18 years ago
Moving non nsbeta1+ XUL 1.0 bugs to mozilla1.2
Target Milestone: mozilla1.0.1 → mozilla1.2

Updated

18 years ago
No longer blocks: 70753

Comment 9

17 years ago
--> default owner
Assignee: hyatt → jaggernaut
Status: ASSIGNED → NEW
Target Milestone: mozilla1.2alpha → ---

Updated

17 years ago
Attachment #53196 - Flags: needs-work+
Comment on attachment 53196 [details] [diff] [review]
patch to add orient="horizontal" to <arrowscrollbox/>

I didn't think the xbl changes were necessary although I agree some skinning is
necessary.

Updated

16 years ago
Keywords: mozilla0.9.8
What happened to this bug? A patch was being developed, now almost 3 years later
nothing more has happened?

In the meantime horizontal arrowscrollboxes do now work, however the scroll
left/right buttons still have arrows pointing up and down respectively.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050511
Firefox/1.0.4

Comment 12

14 years ago
bugzilla.20.jomel@spamgourmet.com: it took nearly a year to get a review
comment, it's quite likely bugzilla@lojjic.net gave up years ago, if you want to
adopt it, just post a new patch and ask someone on irc.mozilla.org for help
getting reviews
Updates all (6) official themes to use left and right arrows for
autorepeatbuttons, unless they are children of an arrowscrollbox with
orient=vertical, in which case they use the up and down arrows.

Note: for compatibility with existing themes I kept the classnames of the two
autorepeatbuttons as autorepeatbutton-up and autorepeatbutton-down, even though
-back and -forward would make more sense. If we wanted to gradually change the
classnames the best option might be to set both classnames on the
autorepeatbuttons for a while, e.g. <autorepeatbutton
class="autorepeatbutton-back autorepeatbutton-up" (...) >

There are potentially one or two minor css adjustments that need doing as some
of the padding etc. might be optimized for autorepeatbuttons in vertical
arrowscrollboxes, but those are details which can be tweaked later and I'm not
the best person to be doing them.

Updated

14 years ago
Attachment #186140 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 186140 [details] [diff] [review]
Makes arrows on horizontal arrowscrollboxes horizontal

These should all say orient="vertical" with quotes. r=me with this fixed.

By comparison, scrollbars seem to use type="increment" and type="decrement"
even though the buttons have up and down classes too.
Attachment #186140 - Flags: review?(neil.parkwaycc.co.uk) → review+
Attachment #186140 - Attachment is obsolete: true

Updated

14 years ago
Attachment #186856 - Flags: review?(mconnor)
Posted patch Fix minor bit-rot (obsolete) — Splinter Review
Just to fix minor bit-rot. This has been sitting around for ages...
Attachment #53196 - Attachment is obsolete: true
Attachment #186856 - Attachment is obsolete: true
Attachment #217113 - Flags: review?(mconnor)
Attachment #186856 - Flags: review?(mconnor)
This was fixed in Firefox (winstripe and pinstripe) by sspitzer in bug 318168 [1].

However later in the same bug mano modified it [2], assuming that the default orientation of an arrowscrollbox is vertical, when in fact it seems to be horizontal [3]. So now an arrowscrollbox with no orientation (hence horizontal) will have arrows in the wrong direction (vertical) [3]. Though other changes mean that such an arrowscrollbox will also now fail to scroll anyway!

Seamonkey still just has the old (arrows always vertical) behaviour.

I can make an updated patch for Firefox and/or Seamonkey if desired.

[1]: https://bugzilla.mozilla.org/attachment.cgi?id=227121&action=diff#mozilla/toolkit/themes/pinstripe/global/scrollbox.css_sec1

[2]: https://bugzilla.mozilla.org/attachment.cgi?id=228077&action=diff#mozilla/toolkit/themes/pinstripe/global/scrollbox.css_sec1

[3]: data:application/vnd.mozilla.xul+xml,<?xml version="1.0"?><!DOCTYPE window><window orient="vertical" xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"><arrowscrollbox><button label="These"/><button label="buttons"/><button label="are"/><button label="laid"/><button label="out"/><button label="horizontally"/></arrowscrollbox></window>

Updated

12 years ago
Attachment #217113 - Attachment is obsolete: true
Attachment #217113 - Flags: review?(mconnor)
John in comment #17
> ...
> I can make an updated patch for Firefox and/or Seamonkey if desired.

John, are you still up for doing that?
QA Contact: jrgmorrison → xptoolkit.widgets

Updated

11 years ago
Assignee: jag → nobody
You need to log in before you can comment on or make changes to this bug.