Steps to reproduce: 1. load bug 763838 (or anything else long, really) 2. hit the [End] key 3. hit the up arrow key 4. hit the down arrow key 5. hit the down arrow key again Expected results: after step 5: no change Actual results: after step 5: some lines shift and others don't I see this on Linux, and have been for a while (months?), and find it pretty annoying.

Regression window in mozilla-central nightlies is: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=654ac86492e8&tochange=e6a9572b48f7

Er, now that I look through the other dependencies of bug 681192, this is probably a duplicate of bug 756400 or bug 767710 (neither of which has "bottom" in the bug summary, but both probably should).

This is probably what's happening. So let's say the maximum scroll position is 6010.4 pixels. Hitting end scrolls us to 6010.4. Hitting the up arrow goes to say 6000.4, but we then round that to the nearest dev pixel and scroll to 6000. Down arrow goes to 6010, down arrow again goes to 6010.4. Is there a reason we don't want to restrict to dev pixels even when scrolling to the end?

(In reply to Timothy Nikkel (:tn) from comment #3) > This is probably what's happening. So let's say the maximum scroll position > is 6010.4 pixels. Hitting end scrolls us to 6010.4. Hitting the up arrow > goes to say 6000.4, but we then round that to the nearest dev pixel and > scroll to 6000. Such rounding should not happen, we should be staying at 6000.4.

I need to come up with a test for this.

Does this also fix it if you scroll from the top rather than using the [End] key? (comment 4 wouldn't, I think, but this seems different.)

Comment on attachment 640925 [details] [diff] [review] fix >+ // Convert the result to layer pixels --- coordinates in the space of >+ // a ThebesLayer. >+ // aCurrentLayerOffset is the coordinates of the top-left of the scrolled frame, >+ // in ThebesLayer space. >+ double layerVal = (aRes*aDesired)/aAppUnitsPerPixel + aCurrentLayerOffset; Adding aCurrentLayerOffset is the right thing to do here, not subtracting?

(In reply to David Baron [:dbaron] from comment #6) > Does this also fix it if you scroll from the top rather than using the [End] > key? (comment 4 wouldn't, I think, but this seems different.) I'm not sure exactly what you mean. When you scroll down gently to the end, the last scroll operation may indeed scroll by a subpixel amount causing lines to move by a pixel relative to each other. However, that would normally happen in the same repaint as the whole page moves by some number of pixels, so I would expect it to be quite difficult to see.

(In reply to Timothy Nikkel (:tn) from comment #7) > >+ // Convert the result to layer pixels --- coordinates in the space of > >+ // a ThebesLayer. > >+ // aCurrentLayerOffset is the coordinates of the top-left of the scrolled frame, > >+ // in ThebesLayer space. > >+ double layerVal = (aRes*aDesired)/aAppUnitsPerPixel + aCurrentLayerOffset; > > Adding aCurrentLayerOffset is the right thing to do here, not subtracting? Yes. The comment was supposed to make that clear.

https://hg.mozilla.org/integration/mozilla-inbound/rev/9acc5029800e Backed out due to test failures: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=e42786561b60

I pretty much rewrote this patch... https://tbpl.mozilla.org/?tree=Try&rev=4d5112d7c5cc

This is the patch, completely rewritten.

This patch makes the existing test pass on Mac.

Comment on attachment 647027 [details] [diff] [review] fix v2 >diff --git a/gfx/2d/BaseRect.h b/gfx/2d/BaseRect.h >--- a/gfx/2d/BaseRect.h >+++ b/gfx/2d/BaseRect.h This hunk seems unrelated? I can't see it being used anywhere in this patch. Sorry for the delay.

(In reply to Timothy Nikkel (:tn) from comment #16) > >diff --git a/gfx/2d/BaseRect.h b/gfx/2d/BaseRect.h > >--- a/gfx/2d/BaseRect.h > >+++ b/gfx/2d/BaseRect.h > > This hunk seems unrelated? I can't see it being used anywhere in this patch. Good point. Removed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/6424adfb7ac2 https://hg.mozilla.org/integration/mozilla-inbound/rev/7566f863512b https://hg.mozilla.org/integration/mozilla-inbound/rev/313b5b97e7e3

Backed out: https://hg.mozilla.org/integration/mozilla-inbound/rev/e63fd4fc05a0 for crashes like: https://tbpl.mozilla.org/php/getParsedLog.php?id=14126158&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=14126208&tree=Mozilla-Inbound

Thanks. Probably just a silly bug. We do this: if (f->GetStateBits() & NS_FRAME_HAS_CONTAINER_LAYER) { nsTArray<DisplayItemData>* array = GetDisplayItemDataArrayForFrame(f); And assume array is non-null. Of course it's possible for a frame to have NS_FRAME_HAS_CONTAINER_LAYER and not have any display items ... the root frame will probably fall into this category! https://tbpl.mozilla.org/?tree=Try&rev=ac925b233cac

https://hg.mozilla.org/integration/mozilla-inbound/rev/f89ae41eed63 https://hg.mozilla.org/integration/mozilla-inbound/rev/b15fb3603bfe https://hg.mozilla.org/integration/mozilla-inbound/rev/14d17919e235 https://hg.mozilla.org/integration/mozilla-inbound/rev/6ea008b301da

This caused bug 776381 to go perma-orange (native Android mochitest-7), so has unfortunately had to be backed out. https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=Android%20Tegra%20250%20mozilla-inbound%20opt%20test%20mochitest-7&rev=6ea008b301da Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/4e8871b8e913

It seems test_offsets.js fails because the mochitest viewport has been zoomed, causing the scrolling code to select a subpixel offset for the scroll operation. Rounding the change in getBoundingClientRect().top/.left should hide the subpixel choices and make the test pass. https://tbpl.mozilla.org/?tree=Try&rev=281dc4834850

https://hg.mozilla.org/integration/mozilla-inbound/rev/8364cb62506e https://hg.mozilla.org/integration/mozilla-inbound/rev/b2127fc9bd2b https://hg.mozilla.org/integration/mozilla-inbound/rev/1ee6a1ae6cfc https://hg.mozilla.org/integration/mozilla-inbound/rev/970496fa31dd https://hg.mozilla.org/integration/mozilla-inbound/rev/6efd09dda9e1

https://hg.mozilla.org/mozilla-central/rev/8364cb62506e https://hg.mozilla.org/mozilla-central/rev/b2127fc9bd2b https://hg.mozilla.org/mozilla-central/rev/1ee6a1ae6cfc https://hg.mozilla.org/mozilla-central/rev/970496fa31dd https://hg.mozilla.org/mozilla-central/rev/6efd09dda9e1

Comment on attachment 647027 [details] [diff] [review] fix v2 Review of attachment 647027 [details] [diff] [review]: ----------------------------------------------------------------- Not very risky, fixes a subtle but annoying issue. Also speeds up scrolling in some situations.

Comment on attachment 647028 [details] [diff] [review] Testcase for this bug Review of attachment 647028 [details] [diff] [review]: ----------------------------------------------------------------- Not very risky, fixes a subtle but annoying issue. Also speeds up scrolling in some situations.

Comment on attachment 647029 [details] [diff] [review] Mark Mac test as passing Review of attachment 647029 [details] [diff] [review]: ----------------------------------------------------------------- Not very risky, fixes a subtle but annoying issue. Also speeds up scrolling in some situations.

Comment on attachment 649033 [details] [diff] [review] fix crash Review of attachment 649033 [details] [diff] [review]: ----------------------------------------------------------------- Not very risky, fixes a subtle but annoying issue. Also speeds up scrolling in some situations.

Comment on attachment 649150 [details] [diff] [review] fix test Review of attachment 649150 [details] [diff] [review]: ----------------------------------------------------------------- Not very risky, fixes a subtle but annoying issue. Also speeds up scrolling in some situations.

This has been approved for Aurora for 5 days, please land to that branch. Wontfixing for Beta at this point since we've already gone to build for beta 5 and we won't take this so late in the Beta cycle.

https://hg.mozilla.org/releases/mozilla-aurora/rev/169ec60d32c9 https://hg.mozilla.org/releases/mozilla-aurora/rev/ee69702b3f71 https://hg.mozilla.org/releases/mozilla-aurora/rev/ca10ea6d11ea https://hg.mozilla.org/releases/mozilla-aurora/rev/f1c542259ffc https://hg.mozilla.org/releases/mozilla-aurora/rev/cb0208e24e22