-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Icons: minor edits, additional example #6954
Conversation
Resolves 4758 Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
Resolves #4758 Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
This seems to be a proposal to add a new I don't know who on Chrome works on this area and could give a more official position, but I'd imagine we're negative on such a proposal; the existing |
Thanks @domenic! Could this be in the spec as something “optional,” or is that a bad approach? (I suppose if this was done for everything it would soon clutter the spec though.) Perhaps I better file an issue for the favicon.svg fallback topic, to continue with the other edits here? Would this require a Wattsi build (don’t recall the server error from anything in the past)? |
Generally we try to avoid optional features; see https://whatwg.org/working-mode#changes "Optional or implementation-defined behavior must be very well motivated." But I guess before discussing whether or not this is well-motivated it'd be first worthwhile to see if there's implementer interest, and if so, what those implementers' feelings are on optionality.
Yeah, that might make sense.
I'm not sure what you mean; every spec edit is built by Wattsi. The server error is because you have an invalid patch; as it says
i.e. you seem to be trying to reference a term "urls" which does not exist. (Probably you meant "url"?) |
Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
<p>The following code shows a document specifying one icon.</p> | ||
|
||
<pre><code class="html"><!DOCTYPE HTML> | ||
<title>Un museo fantástico</title> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've been trying to add <html lang=...>
to as many of our examples as possible to encourage good practice. This looks like a good case for doing so since the language is presumably not English like many other examples!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a suggestion or does it need to be changed? I understand your point but it’s not clear to me whether it’s optional or mandatory.
(Just as this is a topic dear to me: I hope when it comes to lang
, we can at some point move from making language declaration a developer duty to making language detection a user agent responsibility. I’m aware this raises concerns and has limits, but it could help a great deal if we identified solutions that are more efficient and reliable.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a request to change.
<link rel=icon href=favicon.svg> | ||
…</code></pre> | ||
|
||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could add a paragraph noting that because there is only one icon, you don't need type or sizes. That confused me for a bit until I thought about it harder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just added one. I wasn’t sure about type
here (wouldn’t that be correctly identified?) but can add something around that, too.
@@ -24961,8 +24971,6 @@ document.body.appendChild(wbr);</code></pre> | |||
<title>lsForums — Inbox</title> | |||
<link rel=icon href=favicon.png sizes="16x16" type="image/png"> | |||
<link rel=icon href=windows.ico sizes="32x32 48x48" type="image/vnd.microsoft.icon"> | |||
<link rel=icon href=mac.icns sizes="128x128 512x512 8192x8192 32768x32768"> | |||
<link rel=icon href=iphone.png sizes="57x57" type="image/png"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why delete these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This goes back to #4758, reflecting the idea of making the spec less suggestive of using many different icons. Taking some out still seems to serve the purpose of showing how one can handle different icon types and sizes, while making it look less like the spec endorsed entirely uninhibited use of them. (No one will care, but hope is this sends a signal of not going for dozens of icons.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather not couple this change with adding a new example, as I don't really agree with the goals of #4758.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would have been useful to know and learn more about in #4758.
Signed-off-by: Jens Oliver Meiert <jens@meiert.com>
Set the PR up poorly; want to revisit and propose a new PR. |
Following up on #4758, this is a proposal to include another example (with just one favicon) and to also promote a favicon.svg fallback.
The suggestion for an additional fallback (as well as the update itself) may be a little naive, so I look forward to learning more how this could be handled.
/links.html ( diff )