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

Update URL to fingerprint image #41

Merged
merged 3 commits into from
Jan 16, 2020
Merged

Conversation

zcorpan
Copy link
Member

@zcorpan zcorpan commented Mar 29, 2017

@zcorpan zcorpan requested a review from annevk March 29, 2017 12:18
@annevk
Copy link
Member

annevk commented Mar 29, 2017

So if we do this and not inline it, we should have maybe two resources on resources.whatwg.org. One for external usage and one for internal usage. WDYT?

@domenic
Copy link
Member

domenic commented Mar 29, 2017

Why would we need two?

@annevk
Copy link
Member

annevk commented Mar 29, 2017

@domenic as discussed in #whatwg, there's no need for ARIA and width/height information in the external variant as <img> will take care of that.

@zcorpan
Copy link
Member Author

zcorpan commented Mar 29, 2017

But having those attribute there doesn't hurt either, it's just wasting a few bytes.

@domenic
Copy link
Member

domenic commented Mar 29, 2017

Hmm, I see. Why would we host an "internal variant" one then? Why not just use <img> everywhere?

@zcorpan
Copy link
Member Author

zcorpan commented Mar 29, 2017

OK I've changed fingerprint.svg to omit the attributes.

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

LGTM to land (after this resource is up, of course)

@annevk
Copy link
Member

annevk commented Sep 23, 2019

Now that this is generalized to tracking vector changes to whatwg/html are needed too: whatwg/html#4933.

Copy link
Contributor

@sideshowbarker sideshowbarker left a comment

Choose a reason for hiding this comment

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

Reviewed and tested

annevk added a commit to whatwg/html that referenced this pull request Jan 7, 2020
annevk added a commit to speced/bikeshed that referenced this pull request Jan 7, 2020
And align with defaults used in whatwg/wattsi#41.
tabatkins pushed a commit to speced/bikeshed that referenced this pull request Jan 13, 2020
And align with defaults used in whatwg/wattsi#41.
@annevk
Copy link
Member

annevk commented Jan 15, 2020

@sideshowbarker is this still okay? Need to make a few tweaks to the HTML PR but then this is ready to go.

annevk added a commit to whatwg/html that referenced this pull request Jan 15, 2020
Copy link
Contributor

@sideshowbarker sideshowbarker left a comment

Choose a reason for hiding this comment

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

is this still okay? Need to make a few tweaks to the HTML PR but then this is ready to go.

Still okay

@annevk annevk merged commit a931147 into master Jan 16, 2020
@annevk annevk deleted the zcorpan/update-fingerprint branch January 16, 2020 15:46
annevk added a commit to whatwg/html that referenced this pull request Jan 16, 2020
annevk added a commit to whatwg/html that referenced this pull request Jan 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants