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

Port chrome-only dialog tests to WPT part 4 #31889

Merged
merged 1 commit into from
Dec 12, 2021

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Dec 4, 2021

Bug: 1240798
Change-Id: I6267d6ce10f1cdf3b326b8a8cbbf90054a47f658
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3314613
Reviewed-by: Mason Freed <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#950887}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

The review process for this patch is being conducted in the Chromium project.

@@ -0,0 +1,72 @@
<!DOCTYPE html>
<link rel=author href="mailto:[email protected]">
Copy link
Member

Choose a reason for hiding this comment

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

nit: Here and other tests, would be nice if double quotes were used around the attr value.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've been doing rel=author instead of rel="author" for years, is there a style guide or anything? I kind of like not using quotes with simple string values.

Copy link
Member

Choose a reason for hiding this comment

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

not sure if there's a style guide (I wish there was), but attribute values most commonly have double quotes around them, not sure why smaller values should be exempt.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, ill add quotes to all the dialog tests as a followup

Copy link
Contributor

Choose a reason for hiding this comment

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

I created a followup to do this: #32043

Bug: 1240798
Change-Id: I6267d6ce10f1cdf3b326b8a8cbbf90054a47f658
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3314613
Reviewed-by: Mason Freed <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#950887}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants