Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(108)

Issue 1875943004: [DNS] Don't adjust the scroll position if the anchor element changes in size (Closed)

Created:
4 years, 8 months ago by ymalik
Modified:
4 years, 7 months ago
Reviewers:
CC:
blink-reviews, blink-reviews-layout_chromium.org, chromium-reviews, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, szager+layoutwatch_chromium.org, zoltan1
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Don't adjust the scroll position if the anchor element changes in size https://crrev.com/1755923002 added logic to pick a corner of the anchor candidate from which the relative offset would be calculated. Even if the anchor element itself changed in height (during restore), the scroll position would be adjusted accordingly. This breaks for cases like crbug.com/600891 where the content is added to the anchor node and adjusting the scroll to compensate for the change is not the right thing to do. Note that the logic above was added with a bunch of other changes and it's not clear whether or not it was done to fix any existing bugs. i.e. we may need to revisit the changes in this CL if we find that it doesn't work in some cases. BUG=600891

Patch Set 1 #

Patch Set 2 : Don't adjust offset if size of anchor object changewd #

Patch Set 3 : nit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+55 lines, -21 lines) Patch
M third_party/WebKit/Source/core/layout/ScrollAnchor.h View 1 2 1 chunk +4 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/ScrollAnchor.cpp View 1 2 chunks +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/ScrollAnchorTest.cpp View 1 3 chunks +45 lines, -20 lines 0 comments Download

Messages

Total messages: 6 (4 generated)
ymalik
@ojan, PTAL :)
4 years, 8 months ago (2016-04-12 20:13:44 UTC) #4
ojan
4 years, 8 months ago (2016-04-13 17:50:11 UTC) #5
I think we should wait until skobes is back from vacation. I suspect that this
is working around the fact that our current anchoring algorithm is wrong. It'll
be hard to know if this is a good change until we have a good understanding of
what the anchoring algorithm should be.

Powered by Google App Engine
This is Rietveld 408576698