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.

Sign up for GitHub

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

Jump to bottom

HTML: Add a historical test for the search event and incremental="" #19889

Merged
merged 2 commits into from Oct 29, 2019
Merged

HTML: Add a historical test for the search event and incremental="" #19889

merged 2 commits into from Oct 29, 2019

Conversation

zcorpan
Copy link
Member

@zcorpan zcorpan commented Oct 25, 2019

@zcorpan zcorpan requested a review from foolip October 25, 2019 14:40
@zcorpan zcorpan mentioned this pull request Oct 25, 2019
Closed
@zcorpan
Copy link
Member Author

zcorpan commented Oct 25, 2019

Test results:
https://wpt.fyi/results/html/semantics/forms?label=pr_head&max-count=1&pr=19889

domenic
domenic approved these changes Oct 25, 2019
View reviewed changes
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as tests, and I guess we should feel free to merge tests that reflect the current spec. But it's not clear to me what the future is for these; in particular the search event seems like it might need to get specced based on the use counter discussions.

@zcorpan
Copy link
Member Author

zcorpan commented Oct 25, 2019

@tkent-google do you think this should be merged now or wait until we know if removal is successful?

@zcorpan zcorpan assigned tkent-google and unassigned zqzhang Oct 25, 2019
tkent-google
tkent-google approved these changes Oct 28, 2019
View reviewed changes
Copy link
Contributor

@tkent-google tkent-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think this should be merged now or wait until we know if removal is successful?

I think Chrome can remove them, and we can merge this PR now.

@zcorpan zcorpan merged commit 3b93e83 into master Oct 29, 2019
@zcorpan zcorpan deleted the zcorpan/historical-onsearch branch October 29, 2019 21:10
@zcorpan
Copy link
Member Author

zcorpan commented Oct 29, 2019

Thanks @tkent-google

@zcorpan zcorpan mentioned this pull request Oct 29, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@domenic domenic domenic approved these changes

@tkent-google tkent-google tkent-google approved these changes

@foolip foolip Awaiting requested review from foolip

@annevk annevk Awaiting requested review from annevk

@jdm jdm Awaiting requested review from jdm

@jgraham jgraham Awaiting requested review from jgraham

@zqzhang zqzhang Awaiting requested review from zqzhang

Assignees

@tkent-google tkent-google

Labels
html
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@zcorpan @domenic @tkent-google @zqzhang @wpt-pr-bot @moz-wptsync-bot