Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1288693003:
Fix scroll-restoration-push-replace.html test to support scrollTopLeftInterop mode (Closed)
|
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 |
