Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make endScrollOffset check inclusive for max-scroll-position #37

Merged
merged 5 commits into from Feb 22, 2019

Conversation

Projects
None yet
4 participants
@stephenmcgruer
Copy link
Collaborator

commented Feb 5, 2019

See the issue for more details

Fixes #19

Make endScrollOffset check inclusive for max-scroll-position
See the issue for more details

Fixes #19

@stephenmcgruer stephenmcgruer requested review from majido and flackr Feb 5, 2019

@stephenmcgruer

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 5, 2019

Rob, Majid - I'm having trouble finding a way to word this change. This is what I was able to come up with but it feels awkward. Before I send it to the other spec owners, can you take a look and see if you can think of a better way to word it?

Show resolved Hide resolved Overview.bs

@stephenmcgruer stephenmcgruer requested review from birtles and graouts Feb 11, 2019

@birtles
Copy link
Collaborator

left a comment

r=me with the suggested tweaks

Show resolved Hide resolved Overview.bs Outdated

stephenmcgruer added some commits Feb 12, 2019

@stephenmcgruer

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 12, 2019

@birtles It seems like bikeshed on Travis is (somehow?) segfaulting - https://travis-ci.org/WICG/scroll-animations/jobs/492189790. I cannot reproduce locally, do you have any idea how we could debug this?

@birtles

This comment has been minimized.

Copy link
Collaborator

commented Feb 13, 2019

@birtles It seems like bikeshed on Travis is (somehow?) segfaulting - https://travis-ci.org/WICG/scroll-animations/jobs/492189790. I cannot reproduce locally, do you have any idea how we could debug this?

Wow, that's interesting. I wonder if it's one of the dependencies it's picking up.

@tabatkins Are there any flags we can pass to bikeshed to get debug information?

@birtles

This comment has been minimized.

Copy link
Collaborator

commented Feb 13, 2019

I wonder if it's the requirements.txt added in tabatkins/bikeshed@d76ab51#diff-b4ef698db8ca845e5845c4618278f29a that is causing travis to pick up different versions of the dependencies and one of those is crashing.

@stephenmcgruer stephenmcgruer merged commit 54383b7 into WICG:master Feb 22, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
ipr PR deemed acceptable.
Details

@stephenmcgruer stephenmcgruer deleted the stephenmcgruer:end_inclusive branch Feb 22, 2019

birtles pushed a commit that referenced this pull request Feb 22, 2019

Bikeshed update for 'Make endScrollOffset check inclusive for max-scr…
…oll-position (#37)

See the issue for more details

Fixes #19
' [ci skip]

Generated from:

commit 54383b7
Author: Stephen McGruer <stephen.mcgruer@gmail.com>
Date:   Fri Feb 22 15:26:32 2019 -0500

    Make endScrollOffset check inclusive for max-scroll-position (#37)

    See the issue for more details

    Fixes #19
@tabatkins

This comment has been minimized.

Copy link
Collaborator

commented Feb 22, 2019

The segfault should be avoided now. The latest version of LXML has a segfault in some common code that Bikeshed happens to trip in a lot of specs. The first attempt at writing requirements.txt didn't use the right syntax, so it didn't avoid the problem, but now it should be correct. ^_^

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this pull request Feb 27, 2019

ScrollTimeline: make endScrollOffset inclusive only when max-scroll-p…
…osition

Before this CL, we always treated endScrollOffset as inclusive, against
the spec. The spec was recently updated to treat endScrollOffset as
inclusive but only when it is equal to max-offset:
WICG/scroll-animations#37

Bug: 934989
Change-Id: I7cd9cf4619e804a54cef33ef8d3ec7395166bace

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this pull request Feb 27, 2019

ScrollTimeline: make endScrollOffset inclusive only when max-scroll-p…
…osition

Before this CL, we always treated endScrollOffset as inclusive, against
the spec. The spec was recently updated to treat endScrollOffset as
inclusive but only when it is equal to max-offset:
WICG/scroll-animations#37

Bug: 934989
Change-Id: I7cd9cf4619e804a54cef33ef8d3ec7395166bace

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this pull request Mar 1, 2019

ScrollTimeline: make endScrollOffset inclusive only when max-scroll-p…
…osition

Before this CL, we always treated endScrollOffset as inclusive, against
the spec. The spec was recently updated to treat endScrollOffset as
inclusive but only when it is equal to max-offset:
WICG/scroll-animations#37

Bug: 934989
Change-Id: I7cd9cf4619e804a54cef33ef8d3ec7395166bace

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this pull request Mar 1, 2019

ScrollTimeline: make endScrollOffset inclusive only when max-scroll-p…
…osition

Before this CL, we always treated endScrollOffset as inclusive, against
the spec. The spec was recently updated to treat endScrollOffset as
inclusive but only when it is equal to max-offset:
WICG/scroll-animations#37

Bug: 934989
Change-Id: I7cd9cf4619e804a54cef33ef8d3ec7395166bace

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this pull request Mar 1, 2019

ScrollTimeline: make endScrollOffset inclusive only when max-scroll-p…
…osition

Before this CL, we always treated endScrollOffset as inclusive, against
the spec. The spec was recently updated to treat endScrollOffset as
inclusive but only when it is equal to max-offset:
WICG/scroll-animations#37

Bug: 934989
Change-Id: I7cd9cf4619e804a54cef33ef8d3ec7395166bace
Reviewed-on: https://chromium-review.googlesource.com/c/1483682
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636673}

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this pull request Mar 1, 2019

ScrollTimeline: make endScrollOffset inclusive only when max-scroll-p…
…osition

Before this CL, we always treated endScrollOffset as inclusive, against
the spec. The spec was recently updated to treat endScrollOffset as
inclusive but only when it is equal to max-offset:
WICG/scroll-animations#37

Bug: 934989
Change-Id: I7cd9cf4619e804a54cef33ef8d3ec7395166bace
Reviewed-on: https://chromium-review.googlesource.com/c/1483682
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636673}

aarongable pushed a commit to chromium/chromium that referenced this pull request Mar 1, 2019

ScrollTimeline: make endScrollOffset inclusive only when max-scroll-p…
…osition

Before this CL, we always treated endScrollOffset as inclusive, against
the spec. The spec was recently updated to treat endScrollOffset as
inclusive but only when it is equal to max-offset:
WICG/scroll-animations#37

Bug: 934989
Change-Id: I7cd9cf4619e804a54cef33ef8d3ec7395166bace
Reviewed-on: https://chromium-review.googlesource.com/c/1483682
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636673}

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 1, 2019

Bug 1531935 [wpt PR 15575] - ScrollTimeline: make endScrollOffset inc…
…lusive only when max-scroll-position, a=testonly

Automatic update from web-platform-tests
ScrollTimeline: make endScrollOffset inclusive only when max-scroll-position

Before this CL, we always treated endScrollOffset as inclusive, against
the spec. The spec was recently updated to treat endScrollOffset as
inclusive but only when it is equal to max-offset:
WICG/scroll-animations#37

Bug: 934989
Change-Id: I7cd9cf4619e804a54cef33ef8d3ec7395166bace
Reviewed-on: https://chromium-review.googlesource.com/c/1483682
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636673}

--

wpt-commits: ce50d58dd4a10d34bd70242d3833e6940169a246
wpt-pr: 15575

mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Apr 2, 2019

Bug 1531935 [wpt PR 15575] - ScrollTimeline: make endScrollOffset inc…
…lusive only when max-scroll-position, a=testonly

Automatic update from web-platform-tests
ScrollTimeline: make endScrollOffset inclusive only when max-scroll-position

Before this CL, we always treated endScrollOffset as inclusive, against
the spec. The spec was recently updated to treat endScrollOffset as
inclusive but only when it is equal to max-offset:
WICG/scroll-animations#37

Bug: 934989
Change-Id: I7cd9cf4619e804a54cef33ef8d3ec7395166bace
Reviewed-on: https://chromium-review.googlesource.com/c/1483682
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636673}

--

wpt-commits: ce50d58dd4a10d34bd70242d3833e6940169a246
wpt-pr: 15575
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.