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

BREAKING CHANGE: Replace "badge" with "monochrome" #833

Merged
merged 17 commits into from
Jun 18, 2020

Conversation

NotWoods
Copy link
Member

@NotWoods NotWoods commented Dec 6, 2019

Closes #830

This change (choose one):

  • Breaks existing normative behavior (please add label "breaking")
  • Adds new normative requirements
  • Adds new normative recommendations or optional items
  • Makes only editorial changes (only changes informative sections, or
    changes normative sections without changing behavior)
  • Is a "chore" (metadata, formatting, fixing warnings, etc).

Implementation commitment (delete if not making normative changes):

Commit message:

Following discussion in #830, renaming purpose: "badge" to purpose: "monochrome" and expanding on the definition.


Preview | Diff

@NotWoods
Copy link
Member Author

NotWoods commented Dec 6, 2019

@mgiuca ready for review

@aarongustafson
Copy link
Collaborator

Should this be filed against ImageResource instead?

@NotWoods
Copy link
Member Author

NotWoods commented Dec 6, 2019

@aarongustafson I believe that right now, all the "purpose" entries are only in the manifest spec.

@aarongustafson
Copy link
Collaborator

@aarongustafson I believe that right now, all the "purpose" entries are only in the manifest spec.

You’re right, I forgot about that. I just got back from a week off, brain is still booting up :-)

@marcoscaceres
Copy link
Member

Before I forget, make sure you also update the ECHIDNA file with the new image:
https://github.com/w3c/manifest/blob/gh-pages/ECHIDNA

Copy link
Member

@marcoscaceres marcoscaceres left a comment

Choose a reason for hiding this comment

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

Some drive by comments... I'm a bit concerned about conflating what the OS will handle and what the browser handles when it comes to these images.

Also, just bike-shedding, "monochrome" seems a bit weird as a "purpose". Is that normal nomenclature across different OS? Do we know what Windows uses or iOS?

Irrespective of the above, looks like it's taking shape tho! Nice one, @NotWoods.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@aarongustafson
Copy link
Collaborator

Also, just bike-shedding, "monochrome" seems a bit weird as a "purpose". Is that normal nomenclature across different OS? Do we know what Windows uses or iOS?

Agreed it does feel odd. Tintable? Silhouette? Shape? Simple? Mark? Imprint?

@NotWoods
Copy link
Member Author

Some other ideas for the purpose:

  • monochrome (used by Android)
  • monochromatic
  • single_color (used by Safari)
  • shady

shape seems like it could be confused with maskable.
simple and mark aren't clear terms, so I think they have the same issues as badge.

I lean towards some variation of "single color" (monochrome/monochromatic/single_color) since that has the most clarity.

@marcoscaceres
Copy link
Member

I'm kinda like "single-color"... it doesn't imply that it will be monochrome... just a single color.

@NotWoods
Copy link
Member Author

Technically monochrome means single color and not greyscale 😉 Though single_color is probably more intuitive.

@marcoscaceres
Copy link
Member

TIL (just wikipedia-educated myself). I've always thought of it as greyscale. I checked what Google images returns for monochrome, and it seems the zeitgeist appears to be "grayscale".

index.html Outdated Show resolved Hide resolved
Copy link
Collaborator

@mgiuca mgiuca left a comment

Choose a reason for hiding this comment

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

Just had a look over the PR.

Looks great, but I want to discuss the greyscale vs monochrome thing, as well as terminology.

Firstly (ignoring the terminology for a moment): it was my intention when we discussed this earlier that the icon would actually be greyscale (i.e., the user agent respects all shades of alpha), not pure black/white (i.e., the user agent treats 0 as transparent and >0 as opaque). The intention of this is to allow for antialiasing.

Let's say the site uploads this icon:
image

Most of the pixels in this icon have alpha 0 or alpha 255, but some around the edges are semitransparent. According to the current text, user agents are required to treat all alphas >0 as fully opaque, which means they MUST render it like this:

image

Note the jagged edges and also the fact that there's "bleeding" across thin gaps, which is due to the fact that even an alpha value of 1 (out of 255) is treated as opaque.

So I think we should actually specify this as a greyscale, not as a pure black/white image. That does give sites the option of having semi-transparent parts of the picture, which is not really the intention, but I don't think that's a serious problem if it happens. We could non-normatively encourage developers to have most of the non-transparent pixels be fully opaque, using partial transparency only for anti-aliasing.

That means replacing this sentence:

If it has alpha equal to zero, the user agent SHOULD NOT display it. If it the alpha component is greater than zero, the user agent SHOULD display it with any tint.

with (suggestion):

The user agent SHOULD display each pixel with its original alpha value, but with a red, green and blue value of the user agent's choosing. It is RECOMMENDED that the user agent use the same color value for all pixels.

You could also say explicitly "User agents SHOULD NOT convert the alpha channel to a 1-bit (on/off) image where each pixel is either fully transparent or fully opaque, because the image may contain partially transparent pixels for anti-aliasing purposes." (On some platforms, it may be necessary to do this, but implementations should avoid it unless necessary, which matches the definition of SHOULD NOT.)

Now onto the terminology. Given what I said above, "greyscale" would indeed be the most technically correct term. So I would be happy with that. My preference, however, is "monochrome", since that best conveys the intention. Even though we allow partially transparent pixels for anti-aliasing, we intend these icons to more or less contain fully transparent and fully opaque pixels.

I do not like "single_color". It has the same meaning as "monochrome" (with the same potential ambiguities) but "monochrome" is an industry standard term (in computer graphics) whereas "single-color" feels like we're dumbing that term down for 6-year-olds. It's harder to type (with the underscore), harder to say, and doesn't really buy us anything in terms of clarity. (i.e., if you were confused by "monochrome" thinking for example it meant a black-and-white image as opposed to an alpha image, then calling it "single-color" would not alleviate that confusion.)

So my votes, in order, are:

  1. monochrome
  2. greyscale
  3. single_color

I'm also not clear on why the spec uses the other term "solid color" to mean the same thing. Can we use one term throughout?

index.html Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@NotWoods
Copy link
Member Author

NotWoods commented Jan 20, 2020

My intention is to allow for anti-aliasing, so I'll make adjustments using your suggestions.

I don't want to use greyscale, because that implies that grey is treated as 50% opacity when the colour has no effect on the displayed image. I'm happy with monochrome but I can also see that developers will read it as "black and white" rather than all one color (pink, blue, etc).

I don't think there's any harm in allowing large semi-transparent areas in the icons. This is already a practice in Android's notification icons (such as in Sync for Reddit) where it helps distinguish parts of a logo where different colours are normally used.

Sync for Reddit notification and logo

@NotWoods
Copy link
Member Author

Throwing another name idea onto the pile: stamp. The website provides the stamp and the user agent provides the "ink pad" used to color it. No black-and-white association.

@leolux
Copy link

leolux commented May 10, 2020

How about setting a color value for the notification color:?
image

@NotWoods
Copy link
Member Author

The color value is already set by theme_color in Firefox. I don't think the spec can dictate that UI, so it's up to the other browser teams to do something similar. We don't need a new color field for the notification.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated
color</dfn>, where only the transparency of the icon can be
controlled. As web applications need to across multiple
platforms, it is possible to indicate that an icon can have a
user-agent-specified color applied by adding the <a>single-color</a>
Copy link
Collaborator

Choose a reason for hiding this comment

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

User Agents can use different colors depending on where they display it - they could even use a gradient - it is really about masking that allows them to do so

Copy link
Collaborator

Choose a reason for hiding this comment

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

For this reason the name "single-color" might be wrong.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you have any preferences on the names previously suggested? (monochrome, greyscale, stamp)

Copy link
Collaborator

@kenchris kenchris May 26, 2020

Choose a reason for hiding this comment

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

The purpose of this icon is that it will be use as monochrome - sounds wrong.

Maybe silhouette? (I guess stamp also works, but we are not actually stamping anything :))

Copy link
Collaborator

Choose a reason for hiding this comment

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

Apart from this I like monochrome more than single-color

Copy link
Collaborator

@kenchris kenchris May 26, 2020

Choose a reason for hiding this comment

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

Maybe "mask" or "image-mask" might be the most descriptive - or "cut-out"

Copy link
Collaborator

Choose a reason for hiding this comment

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

cut-out

  1. a shape cut out of board or another material.
    "a life-size cut-out"
  2. a hole cut in something for decoration or to allow the insertion of something else.
    "the mains output socket is fitted into a suitable cut-out on the rear panel"

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that mask, image-mask, and cut-out could be confused with maskable. Image masking is like cutting out parts of the image that aren't inside the mask. I'd like to steer away from mask/cut terminology.

Similarily, I think you could argue "the purpose of this icon is that it will be used as maskable" sounds just as odd. I think you could rephrase it as:

  • The purpose of this icon is that it is maskable
  • The purpose of this icon is that it is monochrome

I like silhouette but it doesn't fit that dichotomy. I'm going to switch back to monochrome as that has the most consensus.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@NotWoods
Copy link
Member Author

NotWoods commented Jun 3, 2020

I've rebased and switched back to the "monochrome" name. Let me know if there are any more changes that need to be made.

Copy link
Collaborator

@mgiuca mgiuca left a comment

Choose a reason for hiding this comment

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

Looks great, Tiger!

Just a few nits on the writing style. Ready to merge.

index.html Outdated Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Copy link
Member

@marcoscaceres marcoscaceres left a comment

Choose a reason for hiding this comment

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

Couple of suggestions... otherwise looks great.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Show resolved Hide resolved
@marcoscaceres
Copy link
Member

@dominickng @mgiuca, or other potential implementers, we are planning to support this in Firefox, but we'd like a second implementer before merging into the spec.

Is this something you'd be interested in supporting at some point (and thus something you would also liked merged into the spec)?

@mgiuca
Copy link
Collaborator

mgiuca commented Jun 17, 2020

I'm not sure I can say for sure that we're going to use this, but I'd like us to wherever it makes sense (e.g., in notifications).

We currently parse the badge purpose, but we don't seem to use it anywhere (it might be that we use it on our server). I need to confirm that it isn't going to break any sites if we stop parsing "badge".

If I can confirm that, then I would be happy to change our parser to parse "monochrome" instead. I can't commit to actually using it, but I don't think that should stop us landing this change (it'd be going from one unused-in-Chrome field to another, so the "two implementor" rule isn't being any more violated than it is now). Perhaps as a separate issue we would look at offloading it from the CR version of the spec if there aren't two implementations at that time, but I don't think it should stop this landing.

@marcoscaceres
Copy link
Member

Ok, sounds like a plan. And yeah, we can mark it "at risk" during CR if need be.

@marcoscaceres
Copy link
Member

@NotWoods, just need you to rejoin the working group: https://www.w3.org/2004/01/pp-impl/114929/join

@mgiuca
Copy link
Collaborator

mgiuca commented Jun 18, 2020

Confirmed that Chrome does not use purpose: badge anywhere, including on the server-side infrastructure. From our perspective, we are good to rename it and change its semantics. LGTM.

@marcoscaceres marcoscaceres merged commit 2c55d86 into w3c:gh-pages Jun 18, 2020
@NotWoods NotWoods deleted the purpose-monochrome branch June 18, 2020 14:16
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request Jul 15, 2020
Web App Manifest spec term was updated recently:
w3c/manifest#833

Bug: 1096388
Fixes: 1096388
Change-Id: I6fe6da6abb8d4e06391c46d4054a746bc49e6e1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2291512
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Mugdha Lakhani <nator@chromium.org>
Commit-Queue: Glen Robertson <glenrob@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788482}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
Web App Manifest spec term was updated recently:
w3c/manifest#833

Bug: 1096388
Fixes: 1096388
Change-Id: I6fe6da6abb8d4e06391c46d4054a746bc49e6e1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2291512
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Mugdha Lakhani <nator@chromium.org>
Commit-Queue: Glen Robertson <glenrob@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#788482}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 41d3c0fcaf1fe115f85ee3ca304ed9847417829b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename purpose: badge to purpose: monochrome and expand on it
6 participants