|
|
Created:
5 years, 4 months ago by Rick Byers Modified:
5 years, 4 months ago Reviewers:
majidvp CC:
blink-reviews Base URL:
svn://svn.chromium.org/blink/trunk Target Ref:
refs/heads/master Project:
blink Visibility:
Public. |
DescriptionFix scroll-restoration-push-replace.html test to support scrollTopLeftInterop mode
BUG=157855
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200474
Patch Set 1 #Patch Set 2 : Fix whitespace #Messages
Total messages: 12 (4 generated)
rbyers@chromium.org changed reviewers: + majidvp@chromium.org
Majid PTAL. I'm about to flip scrollTopLeftInterop to experimental: https://codereview.chromium.org/1216953003/ (but want this fix in a separate CL in case we need to revert the change to experimental).
Lgtm. It might however be easier to use window.scroll{X, Y} instead to avoid any dependency between these two features. On Wed, Aug 12, 2015, 10:02 PM <rbyers@chromium.org> wrote: > Reviewers: majidvp, > > Message: > Majid PTAL. I'm about to flip scrollTopLeftInterop to experimental: > https://codereview.chromium.org/1216953003/ (but want this fix in a > separate CL > in case we need to revert the change to experimental). > > Description: > Fix scroll-restoration-push-replace.html test to support > scrollTopLeftInterop > mode > > BUG=157855 > > Please review this at https://codereview.chromium.org/1288693003/ > > Base URL: svn://svn.chromium.org/blink/trunk > > Affected files (+3, -3 lines): > M > > LayoutTests/fast/history/scroll-restoration/scroll-restoration-push-replace.html > > > Index: > > LayoutTests/fast/history/scroll-restoration/scroll-restoration-push-replace.html > diff --git > > a/LayoutTests/fast/history/scroll-restoration/scroll-restoration-push-replace.html > > b/LayoutTests/fast/history/scroll-restoration/scroll-restoration-push-replace.html > index > > 3db338f7ca74140e3e322af9c91f3ad537f367cf..4505ac079bb71e906e62fd768c2d09728796f702 > 100644 > --- > > a/LayoutTests/fast/history/scroll-restoration/scroll-restoration-push-replace.html > +++ > > b/LayoutTests/fast/history/scroll-restoration/scroll-restoration-push-replace.html > @@ -58,8 +58,8 @@ > } > console.log(`verifying ${key}`); > assert_equals(history.state.key, key, `state should have key: > ${key}`); > - assert_equals(document.body.scrollLeft, entry.expectedScroll[0], > `scrollLeft is correct for ${key}`); > - assert_equals(document.body.scrollTop, entry.expectedScroll[1], > `scrollTop is correct ${key}`); > + assert_equals(document.scrollingElement.scrollLeft, > entry.expectedScroll[0], `scrollLeft is correct for ${key}`); > + assert_equals(document.scrollingElement.scrollTop, > entry.expectedScroll[1], `scrollTop is correct ${key}`); > > window.history.back(); > })); > @@ -72,4 +72,4 @@ > }, 0); > > }, 'history.{push,replace}State retain and respect > history.scrollRestoration'); > -</script> > \ No newline at end of file > +</script> > > > To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
Yeah document.scrollingElement.scrollTop is synonymous with window.scrollY so it doesn't make any difference either way... On Aug 12, 2015 10:11 PM, "Majid Valipour" <majidvp@google.com> wrote: > Lgtm. It might however be easier to use window.scroll{X, Y} instead to > avoid any dependency between these two features. > > On Wed, Aug 12, 2015, 10:02 PM <rbyers@chromium.org> wrote: > >> Reviewers: majidvp, >> >> Message: >> Majid PTAL. I'm about to flip scrollTopLeftInterop to experimental: >> https://codereview.chromium.org/1216953003/ (but want this fix in a >> separate CL >> in case we need to revert the change to experimental). >> >> Description: >> Fix scroll-restoration-push-replace.html test to support >> scrollTopLeftInterop >> mode >> >> BUG=157855 >> >> Please review this at https://codereview.chromium.org/1288693003/ >> >> Base URL: svn://svn.chromium.org/blink/trunk >> >> Affected files (+3, -3 lines): >> M >> >> LayoutTests/fast/history/scroll-restoration/scroll-restoration-push-replace.html >> >> >> Index: >> >> LayoutTests/fast/history/scroll-restoration/scroll-restoration-push-replace.html >> diff --git >> >> a/LayoutTests/fast/history/scroll-restoration/scroll-restoration-push-replace.html >> >> b/LayoutTests/fast/history/scroll-restoration/scroll-restoration-push-replace.html >> index >> >> 3db338f7ca74140e3e322af9c91f3ad537f367cf..4505ac079bb71e906e62fd768c2d09728796f702 >> 100644 >> --- >> >> a/LayoutTests/fast/history/scroll-restoration/scroll-restoration-push-replace.html >> +++ >> >> b/LayoutTests/fast/history/scroll-restoration/scroll-restoration-push-replace.html >> @@ -58,8 +58,8 @@ >> } >> console.log(`verifying ${key}`); >> assert_equals(history.state.key, key, `state should have key: >> ${key}`); >> - assert_equals(document.body.scrollLeft, entry.expectedScroll[0], >> `scrollLeft is correct for ${key}`); >> - assert_equals(document.body.scrollTop, entry.expectedScroll[1], >> `scrollTop is correct ${key}`); >> + assert_equals(document.scrollingElement.scrollLeft, >> entry.expectedScroll[0], `scrollLeft is correct for ${key}`); >> + assert_equals(document.scrollingElement.scrollTop, >> entry.expectedScroll[1], `scrollTop is correct ${key}`); >> >> window.history.back(); >> })); >> @@ -72,4 +72,4 @@ >> }, 0); >> >> }, 'history.{push,replace}State retain and respect >> history.scrollRestoration'); >> -</script> >> \ No newline at end of file >> +</script> >> >> >> To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
The CQ bit was checked by rbyers@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1288693003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1288693003/20001
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. Only full committers are accepted. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. See http://www.chromium.org/getting-involved/become-a-committer Note that this has nothing to do with OWNERS files.
On 2015/08/13 03:58:52, commit-bot: I haz the power wrote: > No L-G-T-M from a valid reviewer yet. Only full committers are accepted. > Even if an L-G-T-M may have been provided, it was from a non-committer, > _not_ a full super star committer. > See http://www.chromium.org/getting-involved/become-a-committer > Note that this has nothing to do with OWNERS files. lgtm again. I think I responded with the wrong email address.
The CQ bit was checked by majidvp@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1288693003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1288693003/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://src.chromium.org/viewvc/blink?view=rev&revision=200474 |