Closed Bug 817820 Opened 12 years ago Closed 11 years ago

Defect - Cursor should change to reflect that "middle-click scrolling" (autoscroll) is active

Categories

(Firefox for Metro Graveyard :: Input, defect, P2)

All
Windows 8.1
defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: TimAbraldes, Assigned: emtwo)

References

Details

(Whiteboard: [mentor=mbrubeck@mozilla.com][good first bug][lang=css] feature=defect c=tbd u=tbd p=1)

Attachments

(1 file, 2 obsolete files)

On desktop and metro Firefox, middle-clicking causes the browser to enter a state where moving the mouse scrolls the page. On desktop Firefox, we change the cursor to reflect that we are in "scroll up/down" or "scroll up/down/left/right" mode. On Metro Firefox the cursor does not change, so it is not immediately obvious that we have entered this special scrolling mode. We should update the cursor to reflect whether we are in a special "scroll on mouse move" mode.
We need to provide a themed autoscroll element toolkit can lookup via getElementById. Currently it ties to create a popup child window, which fails. http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/browser.xml#850 http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/windows/global/global.css#299
Whiteboard: [metro-mvp?]
Or we simply disable auto-scroll for Metro.
Summary: Cursor should change to reflect that "middle-click scrolling" is active → Cursor should change to reflect that "middle-click scrolling" (autoscroll) is active
Blocks: 855300
No longer blocks: metrov1triage
Disabling the mousescroll invalidates bugs like Bug 811429. Will this be a problem for our testers who don't have touch enabled? Should we kill autoscroll for v1?
Flags: needinfo?(mbrubeck)
All we need here is an image. If we want, we can steal desktops. Don't see why we wouldn't try to get this in, especially since a majority of our initial user base is using the mouse.
Blocks: metrov1triage
No longer blocks: 855300
Whiteboard: [metro-mvp?]
(In reply to :Ally Naaktgeboren from comment #3) > Disabling the mousescroll invalidates bugs like Bug 811429. Auto-scroll and wheel scroll are separate, though I think they share some code/events under the hood. We don't need any images to support wheel scroll. I agree with jimm that this is a good easy polish fix. Probably a P3 or P4, maybe a good starter bug for someone.
Flags: needinfo?(mbrubeck)
Whiteboard: [mentor=mbrubeck@mozilla.com][good first bug][lang=css]
Blocks: metrov1defect&change
No longer blocks: metrov1triage
Priority: -- → P2
Whiteboard: [mentor=mbrubeck@mozilla.com][good first bug][lang=css] → [mentor=mbrubeck@mozilla.com][good first bug][lang=css] feature=defect c=tbd u=tbd p=0
QA Contact: msamuel
Assignee: nobody → msamuel
QA Contact: msamuel
Attachment #760445 - Flags: review?(mbrubeck)
Comment on attachment 760445 [details] [diff] [review] v1: Add middle-click autoscroller icon in metro. Review of attachment 760445 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, this is working great in my testing! I'd just like to change where some of it is implemented in order to reduce coupling a bit. See below for details. ::: browser/metro/theme/browser.css @@ +1031,5 @@ > + background-origin: padding-box; > + background-clip: padding-box; > +%ifdef XP_WIN > + background-position: right top; > +%endif Please remove the ifdef here, and just use the "XP_WIN" style unconditionally. (For Metro Firefox, we don't need to style things differently for non-Windows platforms.) @@ +1040,5 @@ > +%ifdef XP_WIN > + background-position: right center; > +%else > + background-position: left center; > +%endif Same here -- keep the "right center" line and remove the "else" case. And similarly, below. ::: toolkit/content/widgets/browser.xml @@ +575,5 @@ > + if (this._isMetroBrowser) { > + this._autoScrollPopup.hidden = "true"; > + } else { > + this._autoScrollPopup.hidePopup(); > + } I'd like to avoid adding Metro-specific code to this toolkit binding, and I suspect the toolkit peers would too. I'd prefer one of these two approaches: 1) Move the code that touches the autoscroll popup into some isolated methods in toolkit's browser.xml, then override those in Metro's browser.xml, or 2) give our "popup" element a small XBL binding that implements the showPopup and hidePopup methods from nsIDOMXULPopupElement, so it will act like a normal popup and work with the existing browser binding code. I think I like the second option better, but feel free to implement whichever one seems simpler. Let me know if you need any clarification or whatnot.
Attachment #760445 - Flags: review?(mbrubeck) → review-
Implemented the 2nd approach described in the last comment.
Attachment #760445 - Attachment is obsolete: true
Implemented the 2nd approach described in the last comment.
Attachment #761042 - Attachment is obsolete: true
Attachment #761044 - Flags: review?(mbrubeck)
Comment on attachment 761044 [details] [diff] [review] v2: Add middle-click autoscroller icon in metro. Review of attachment 761044 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! ::: browser/metro/base/content/bindings/popup.xml @@ +3,5 @@ > +<bindings xmlns="http://www.mozilla.org/xbl" > + xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"> > + <binding id="element-popup"> > + > + <implementation inherits="nsIPopupBoxObject"> "inherits" should be "implements" here.
Attachment #761044 - Flags: review?(mbrubeck) → review+
Blocks: metrov1it9
No longer blocks: metrov1defect&change
Summary: Cursor should change to reflect that "middle-click scrolling" (autoscroll) is active → Defect - Cursor should change to reflect that "middle-click scrolling" (autoscroll) is active
Whiteboard: [mentor=mbrubeck@mozilla.com][good first bug][lang=css] feature=defect c=tbd u=tbd p=0 → [mentor=mbrubeck@mozilla.com][good first bug][lang=css] feature=defect c=tbd u=tbd p=1
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Mozilla/5.0 (Windows NT 6.2; rv:25.0) Gecko/20130625 Firefox/25.0 Verified that the middle-click autoscroller icon is displayed when mouse middle button is clicked and the page scrolls correctly, both horizontally and vertically
Status: RESOLVED → VERIFIED
This doesn't map to a story we specified because we inherited the feature and have no story for it.
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: