Test 1 pixel dancing by caret moving. Bug 784410 does not fix this problem. Regression window(m-i) Good: http://hg.mozilla.org/mozilla-central/rev/6fe7dd2f8f57 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120510021321 Bad: http://hg.mozilla.org/mozilla-central/rev/b7b6565d12a0 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120510050721 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6fe7dd2f8f57&tochange=b7b6565d12a0 Regression window(m-i) Good: http://hg.mozilla.org/integration/mozilla-inbound/rev/b5304fd23df9 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120509222721 Bad: http://hg.mozilla.org/integration/mozilla-inbound/rev/67091352b7d2 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:15.0) Gecko/15.0 Firefox/15.0a1 ID:20120509223521 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b5304fd23df9&tochange=67091352b7d2 Last good: c3d3bfb3b68d First bad: 9d9a3edaa0b9 Triggered by 9d9a3edaa0b9 Robert O'Callahan — Bug 681192. Part 11: Don't snap scrollrange endpoints to device pixels anymore. r=matspal

Not tracking this for now, as this is already there on 15.0 and we have not heard any user complaints yet & seems to be a very minor issue. Please renominate, if you strongly feel this should be tracked.

This is similar to bug 784410 but involves ScrollToShowRect/ScrollContentIntoView.

This patch fixes it, but I'm not sure it's really the way to go. I'll think about it a little more.

Comment on attachment 662117 [details] [diff] [review] Part 2: Add API to ScrollIntoView methods to control whether to scroll in a direction when that direction is not a perceived scrollable direction > + nsRect scrollRange = aScrollFrame->GetScrollRange(); scrollRange is never used

Comment on attachment 662118 [details] [diff] [review] Part 3: Don't scroll vertically to get the caret into view if that's not a perceived scrollable direction + // Don't scroll vertically to get the caret into view, if it doesn't look + // like that is a scrollable direction for the container (i.e. if there + // is no scrollbar showing, or the allowed scroll range is very small). + aVertical.mScrollEvenWhenNotPerceivedScrollableDirection = false; I had to read this a few times to understand it. There seems to be double negation in both the comment and assignment. Compare this with using positive terms: // Scroll vertically to get the caret into view, but only if the container // is perceived scrollable in that direction (i.e. there is a visible // vertical scrollbar or the scroll range is at least one device pixel). aVertical.mOnlyIfPerceivedScrollableDirection = true; Either way is fine, but I prefer the latter logic. r=mats

Sounds good, I'll do that.

https://hg.mozilla.org/integration/mozilla-inbound/rev/50d89c05af5b https://hg.mozilla.org/integration/mozilla-inbound/rev/6211dca38071 https://hg.mozilla.org/integration/mozilla-inbound/rev/a04c322290d7

Backed out due to the new test failing. https://hg.mozilla.org/integration/mozilla-inbound/rev/d678fc7cfbef

I got a couple of the changes for comment #8 wrong. Relanded with those fixed. https://hg.mozilla.org/integration/mozilla-inbound/rev/46fce5a9890f https://hg.mozilla.org/integration/mozilla-inbound/rev/de04a102395f https://hg.mozilla.org/integration/mozilla-inbound/rev/83737cad6889

https://hg.mozilla.org/mozilla-central/rev/46fce5a9890f https://hg.mozilla.org/mozilla-central/rev/de04a102395f https://hg.mozilla.org/mozilla-central/rev/83737cad6889

Comment on attachment 662116 [details] [diff] [review] Part 1: Add nsIScrollableFrame::GetPerceivedScrollingDirections to consolidate logic for whether UI actions should be allowed to scroll in a given direction Review of attachment 662116 [details] [diff] [review]: ----------------------------------------------------------------- This is a somewhat annoying user-visible regression. I think we should take it on Aurora but not beta.

Comment on attachment 662117 [details] [diff] [review] Part 2: Add API to ScrollIntoView methods to control whether to scroll in a direction when that direction is not a perceived scrollable direction Review of attachment 662117 [details] [diff] [review]: ----------------------------------------------------------------- This is a somewhat annoying user-visible regression. I think we should take it on Aurora but not beta.

Comment on attachment 662118 [details] [diff] [review] Part 3: Don't scroll vertically to get the caret into view if that's not a perceived scrollable direction Review of attachment 662118 [details] [diff] [review]: ----------------------------------------------------------------- This is a somewhat annoying user-visible regression. I think we should take it on Aurora but not beta.

Comment on attachment 662117 [details] [diff] [review] Part 2: Add API to ScrollIntoView methods to control whether to scroll in a direction when that direction is not a perceived scrollable direction Review of attachment 662117 [details] [diff] [review]: ----------------------------------------------------------------- This is a somewhat annoying user-visible regression. I think we should take it on Aurora but not beta.

Comment on attachment 662118 [details] [diff] [review] Part 3: Don't scroll vertically to get the caret into view if that's not a perceived scrollable direction Approving for Aurora since we'll get some good bake time before 17 ships.

https://hg.mozilla.org/releases/mozilla-aurora/rev/97953e11eb50 https://hg.mozilla.org/releases/mozilla-aurora/rev/41f730abab15 https://hg.mozilla.org/releases/mozilla-aurora/rev/e183e7a0c34a

Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/17.0 Firefox/17.0 Verified on Firefox 17 beta 6 that the 1 pixel dancing by caret moving is gone. Verified by using the attached test case from the Description.