Open Bug 103723 Opened 23 years ago Updated 2 years ago

arrowscrollbox - add orient="horizontal"

Categories

(Core :: XUL, defect)

defect

Tracking

()

People

(Reporter: pali, Unassigned)

References

Details

Attachments

(4 obsolete files)

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
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
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.
Keywords: patch
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"
Blocks: 70753
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0.1
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
Keywords: mozilla0.9.8
Adding review not sure why this patch is still sitting around.
Keywords: review
doubt this will make 0.9.8
Keywords: mozilla0.9.9
Blocks: 123569
Moving non nsbeta1+ XUL 1.0 bugs to mozilla1.2
Target Milestone: mozilla1.0.1 → mozilla1.2
No longer blocks: 70753
--> default owner
Assignee: hyatt → jaggernaut
Status: ASSIGNED → NEW
Target Milestone: mozilla1.2alpha → ---
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.
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
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.
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
Attachment #186856 - Flags: review?(mconnor)
Attached patch Fix minor bit-rot (obsolete) (deleted) — 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>
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
Assignee: jag → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: