User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110517 Firefox/6.0a1 Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110517 Firefox/6.0a1 Speed page scrolling does not change when changing these values. Remains equal to 3 lines. Reproducible: Always Steps to Reproduce: 1. Open about:config 2. find mousewheel.withnokey.numlines 3. change to anything number 4. restart OR 1. Open about:config 2. find mousewheel.withnokey.numlines 3. change to anything number 4. find mousewheel.withnokey.sysnumlines 5. change to false 6. restart Actual Results: The scrolling speed is still equal to 3 lines. Expected Results: Scroll speed becomes equal to the value mousewheel.withnokey.numlines.
Confirmed on http://hg.mozilla.org/mozilla-central/rev/f717485edc51 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110517 Firefox/6.0a1 ID:20110517030625 Regression window: Works; http://hg.mozilla.org/mozilla-central/rev/3e7a21049b6c Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110516 Firefox/6.0a1 ID:20110516160657 Fails: http://hg.mozilla.org/mozilla-central/rev/0a1e7ec7e268 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110516 Firefox/6.0a1 ID:20110516172821 Regressed by: 0a1e7ec7e268 Masayuki Nakano — Bug 605648 Support high resolution scrolling on Windows r=jimm+smaug
Created attachment 533201 [details] [diff] [review] Patch v1.0 If the wheel action doesn't scroll contents or the scroll amount is customized, we shouldn't use high resolution scrolling for now. I'll file a follow up bug of this.
Comment on attachment 533201 [details] [diff] [review] Patch v1.0 This is getting complicated. Does the patch which disables pixel scrolling on Windows fix this problem temporarily? I'll try to review this tomorrow.
Created attachment 533549 [details] [diff] [review] Patch v1.0.1 fix a nit, sorry for the spam.
Comment on attachment 533549 [details] [diff] [review] Patch v1.0.1 I found a mistake, I'll post new patch after I check the fix.
Created attachment 533608 [details] [diff] [review] Patch v1.1 Sorry for the delay. I found another bug which has been since first implementation of pixel scrolling. > PRInt32 > nsEventStateManager::ComputeWheelActionFor(nsMouseScrollEvent* aMouseEvent, > PRBool aUseSystemSettings) > { > PRInt32 action = GetWheelActionFor(aMouseEvent); > if (aUseSystemSettings && > (aMouseEvent->scrollFlags & nsMouseScrollEvent::kIsFullPage)) { > action = MOUSE_SCROLL_PAGE; > } > > if (aMouseEvent->message == NS_MOUSE_PIXEL_SCROLL) { > if (action == MOUSE_SCROLL_N_LINES || action == MOUSE_SCROLL_PAGE || > (aMouseEvent->scrollFlags & nsMouseScrollEvent::kIsMomentum)) { > action = MOUSE_SCROLL_PIXELS; > } else { > // Do not scroll pixels when zooming > action = -1; > } > } else if (aMouseEvent->scrollFlags & nsMouseScrollEvent::kHasPixels) { > if (aUseSystemSettings || > action == MOUSE_SCROLL_N_LINES || action == MOUSE_SCROLL_PAGE || > (aMouseEvent->scrollFlags & nsMouseScrollEvent::kIsMomentum)) { > // Don't scroll lines when a pixel scroll event will follow. > // Also, don't do history scrolling or zooming for momentum scrolls. > action = -1; > } > } > > return action; > } The final if should check MOUSE_SCROLL_PAGE. I had realized this at creating non-adhoc patch for bug 605648 but I forgot it.
(In reply to comment #3) > This is getting complicated. Basically, most changes are moving code, not writing newly. > Does the patch which disables pixel scrolling on Windows fix this problem > temporarily? Do you mean "the patch" is for bug 605648? If so, yes, all regressions which have been found are hidden by the patch.
Comment on attachment 533608 [details] [diff] [review] Patch v1.1 We have so many prefs for mousewheel handling that we really should have good tests. I wonder how to write such. Please add a comment how ComputeWheelActionFor and GetWheelActionFor differ. > nsEventStateManager::DoQueryScrollTargetInfo(nsQueryContentEvent* aEvent, > nsIFrame* aTargetFrame) > { > nsMouseScrollEvent* msEvent = aEvent->mInput.mMouseScrollEvent; >+ >+ // Don't use high resolution scrolling when user customize the scrolling >+ // speed. >+ if (!UseSystemScrollSettingFor(msEvent)) { >+ return; >+ } >+ > nsIScrollableFrame::ScrollUnit unit; >- if (msEvent->scrollFlags & nsMouseScrollEvent::kIsFullPage) { >- unit = nsIScrollableFrame::PAGES; >- } else { >- unit = nsIScrollableFrame::LINES; >+ PRBool allowOverrideSystemSettings; >+ switch (ComputeWheelActionFor(msEvent, PR_TRUE)) { >+ case MOUSE_SCROLL_N_LINES: >+ unit = nsIScrollableFrame::LINES; >+ allowOverrideSystemSettings = PR_TRUE; >+ break; >+ case MOUSE_SCROLL_PAGE: >+ unit = nsIScrollableFrame::PAGES; >+ allowOverrideSystemSettings = PR_FALSE; >+ break; >+ default: >+ // Don't use high resolution scrolling when the action doesn't scroll >+ // contents. >+ return; What if the action is MOUSE_SCROLL_PIXELS? In that case allowOverrideSystemSettings should PR_FALSE and unit = nsIScrollableFrame::DEVICE_PIXELS , right? Those fixed, r=me
(In reply to comment #8) > We have so many prefs for mousewheel handling that we really should have > good tests. > I wonder how to write such. I think that we should create a new API for nsDOMWindowUtils for calling native mouse wheel event handler in nsWindow or nsChildView. I think we should create it before we start to work for DOM3 mouse wheel event implementation. > > nsEventStateManager::DoQueryScrollTargetInfo(nsQueryContentEvent* aEvent, > > nsIFrame* aTargetFrame) > > { > > nsMouseScrollEvent* msEvent = aEvent->mInput.mMouseScrollEvent; > >+ > >+ // Don't use high resolution scrolling when user customize the scrolling > >+ // speed. > >+ if (!UseSystemScrollSettingFor(msEvent)) { > >+ return; > >+ } > >+ > > nsIScrollableFrame::ScrollUnit unit; > >- if (msEvent->scrollFlags & nsMouseScrollEvent::kIsFullPage) { > >- unit = nsIScrollableFrame::PAGES; > >- } else { > >- unit = nsIScrollableFrame::LINES; > >+ PRBool allowOverrideSystemSettings; > >+ switch (ComputeWheelActionFor(msEvent, PR_TRUE)) { > >+ case MOUSE_SCROLL_N_LINES: > >+ unit = nsIScrollableFrame::LINES; > >+ allowOverrideSystemSettings = PR_TRUE; > >+ break; > >+ case MOUSE_SCROLL_PAGE: > >+ unit = nsIScrollableFrame::PAGES; > >+ allowOverrideSystemSettings = PR_FALSE; > >+ break; > >+ default: > >+ // Don't use high resolution scrolling when the action doesn't scroll > >+ // contents. > >+ return; > What if the action is MOUSE_SCROLL_PIXELS? > In that case allowOverrideSystemSettings should PR_FALSE and > unit = nsIScrollableFrame::DEVICE_PIXELS , right? Yes, but is it needed? I don't have a plan which I use this event with pixel scroll event. But it's easy, I'll add it before landing.
http://hg.mozilla.org/mozilla-central/rev/d4746fb15403 If you have some suggestion about the documents of each method, I'll modify it on another regression bug's patch.
Setting resolution to Verified Fixed on Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0b3
or
