I use Slack in a pinned tab in Nightly. Slack changes its favicon to show a red circle badge when a user enters a watched keyword or @ pings me. Starting this week (September 18-22 in Nightly 57-58), I noticed that the red badge favicon will be shown even though no one has pinged me or entered a watched keyboard. And then the red badge won't disappear after I mark all messages as read. This problem is very intermittent but seems to happen a couple times a day. I don't have STR. This is probably a Slack bug, not a Firefox bug, but the problem seems new and keeps happening. Maybe it is a favicon caching issue?

I see a similar form of this. Slacks icon changes from light-grey to white when you have any unread messages in any channel and normally changes back to light grey when you view them all. It isn't always changing back for me now.

I just noticed this -- looking at it with devtools, the image is a data URI (for the slack icon + red highlight). So if there's a Firefox bug here, it would be that we're not catching Slack changing this to something else. Would probably be easiest for someone to run a Firefox build from a few weeks ago to see if it's still broken there. (Or see if someone at Mozilla running Release is seeing this?)

Can confirm this happens on Firefox 57.0b2.

I saw this Slack favicon problem in Nightly 58 a couple times on Friday morning (September 22). I switched to Beta 56 and then Dev Edition 57 (a branch of Nightly 57 from September 14) for the afternoon and I didn't seen the problem once. The problem is intermittent, so this doesn't prove this is a regression in Nightly 57 after Dev Edition 57 was branched, but it is suspicious. I don't have STR. I tried adding very common words (like "the") to my Slack keyword watchlist to trigger frequent red badge alerts, but I haven't been able to reproduce the problem yet today.

status-firefox57=affected as per Nigel's comment 3.

STR for me is something like this: * Leave slack tab open and work on a different tab. * Have someone ping you on Slack. * Look at the ping after a bit (I cannot reproduce this bug if I visit the website instantly). The icon doesn't change despite having no unread messages. I've noticed this yesterday and today. Is there anything more I can do to help debug?

(In reply to Nigel Babu [:nigelb] from comment #6) > * Look at the ping after a bit (I cannot reproduce this bug if I visit the > website instantly). The icon doesn't change despite having no unread > messages. Thanks, Nigel! When you say "look at the ping after a bit", roughly how long are you waiting? 10 seconds? 1 minute? Multiple minutes?

This is fun. So I tried to consistently reproduce this (after writing the steps), I couldn't reproduce it anymore. I tried from 10 seconds to a few minutes. Later today, I pinged a google doc to someone and I got the slackbot notification, the red badge was sticky this time. After a refresh of the page (to clear stickyness), I couldn't reproduce it anymore. I can confirm Dolske's comment that when the notification is sticky, the favicon points to a data URI that is correct (i.e. seemingly not a bug from Slack). So, in short the STR isn't consistent.

Is this due to us using the rich icon instead of the favicon?

This problem seems to also happen in Discord https://discordapp.com/ It's a Voice/Text chat app with a focus on gaming, it's fairly popular and mainstream among people who play PC games. For me it seems fairly reproducible and predictable, it seems there is a "delay" in how the favicon updates. Normally when you receive a message but the window is unfocused (or if you're chatting in another room, etc), it should add a little number to the favicon to indicate how many missed messages you have. What happens instead is that the icon remains at zero notifications, but then on the next update (when i click a chat to see the missed message, which should update the favicon from '1' unread message to zero), it suddenly shows a '1' in the favicon, despite the fact that it should be zero. It seems to always be lagging behind by one favicon update. If you had multiple people send you messages, I'm guessing it would update to "1" when it should be "2" or something, and then if you read those messages, then the next time the icon updated, it'd show "2" even if there was only one notification. So it basically shows you the previous update rather than the current one.

I managed to find reliable steps to reproduce this. 1. Log into any slack instance. 2. Go to Slackbot in your direct messages 3. Type "Hi". Slackbot will respond and in the working case the favicon will not change (or will briefly flash the DM red banner for a moment). In the broken case the DM red banner will appear and stick. Detaching the tab at that point will reset the favicon to what it is meant to be. Mozregression gives us https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4dc8f5f9259c1413dcd2755ae01bad8dc8139050&tochange=6d5fe3151e733d6ac818728f44f5985f1aa63f8c

it's unclear whether this is reproducible in nightly where bug 1401777 is fixed.

(In reply to Marco Bonardo [::mak] from comment #13) > it's unclear whether this is reproducible in nightly where bug 1401777 is > fixed. I can reproduce on current nightly and a build from current mozilla-inbound

what are the <link> of the page? Could be they rely on a specific order of favicons, while now we prefer ico/svg.

(In reply to Marco Bonardo [::mak] from comment #15) > what are the <link> of the page? > Could be they rely on a specific order of favicons, while now we prefer > ico/svg. As far as I can tell there is only one link with rel="shortcut icon" and that is always correct and is the first in hte list of icon related link elements of which there are a number. All of the rest are slack's rich icon and not the icon that is being shown in error.

So, the problem is that multiple icons are set in a short amount of time, the current loop keeps the first one, instead of the last one. I can patch this tomorrow (well, the patch is trivial, but it needs a test).

Hmm... I'm assuming "short amount of time" means the page set an icon and dynamically set another icon within 100ms. The new logic from bug 1352459 debounces to avoid passing around each individual icon, and it happens to prefer the first icon it found during the debouncing period. Even if we prefer the later icon of "equal" quality, it seems like this would "break" the page's intent of flashing an icon as the icon changes happening within 100ms results in 1 icon update.

Just out of curiosity, when this is patched, is it going to be merged and uplifted to 57 in time for 57b4 ? (which according to the schedule is supposed to arrive tomorrow, on the 29th) I ask because otherwise this would stay broken in dev edition until 57b5 right?

(In reply to Azari from comment #19) > Just out of curiosity, when this is patched, is it going to be merged and > uplifted to 57 in time for 57b4 ? (which according to the schedule is > supposed to arrive tomorrow, on the 29th) > > I ask because otherwise this would stay broken in dev edition until 57b5 > right? I don't think this bug will get fixed and uplifted in time for 57b4 because we don't have a patch landed in Nightly yet and 57b4's go-to-build deadline is later today.

(In reply to Ed Lee :Mardak from comment #18) > Even if we prefer the later icon of "equal" quality, it seems like this > would "break" the page's intent of flashing an icon as the icon changes > happening within 100ms results in 1 icon update. I'm honestly not sure there's an intent to flash the icon, regardless it doesn't sound particularly useful. I suspect it was more an outcome of the old approach rather than a wanted feature.

(In reply to Azari from comment #19) > I ask because otherwise this would stay broken in dev edition until 57b5 > right? it's likely most favicon fixes will make b5.

I verified the fix on Slack both by using the steps in comment 12 and using /remind on myself to cause a delayed message.

Comment on attachment 8913296 [details] Bug 1402373 - Slack's red badge favicon is shown for no reason and then isn't cleared when all messages are read. https://reviewboard.mozilla.org/r/184666/#review189834 ::: browser/modules/ContentLinkHandler.jsm:240 (Diff revision 1) > return true; > } > > this.ContentLinkHandler = { > init(chromeGlobal) { > - const faviconLoads = new Map(); > + this._faviconLoads = new Map(); Why is this change necessary? This was a locally scoped variable becase each tab causes `init` to be called wiping out any previous `Map`s.

Comment on attachment 8913296 [details] Bug 1402373 - Slack's red badge favicon is shown for no reason and then isn't cleared when all messages are read. https://reviewboard.mozilla.org/r/184666/#review189844 LGTM. Please fix that issue that Mardak left. I believe the local scoped map is necessary to avoid the potential data loss.

(In reply to Ed Lee :Mardak from comment #25) > Why is this change necessary? It's not, it should have been part of another changeset experimenting for the intermittent failure. Let me remove it.

Tracked since it's a new regression.

Comment on attachment 8913296 [details] Bug 1402373 - Slack's red badge favicon is shown for no reason and then isn't cleared when all messages are read. https://reviewboard.mozilla.org/r/184666/#review190006

Pushed by mak77@bonardo.net: https://hg.mozilla.org/integration/autoland/rev/06df09ff7e64 Slack's red badge favicon is shown for no reason and then isn't cleared when all messages are read. r=Mardak,nanj

Comment on attachment 8913296 [details] Bug 1402373 - Slack's red badge favicon is shown for no reason and then isn't cleared when all messages are read. Approval Request Comment [Feature/Bug causing the regression]: Bug 1352459 [User impact if declined]: sites setting favicons dinamically to show their status (like unread messages) won't work properly. [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: not yet, verified in local build [Needs manual test from QE? If yes, steps to reproduce]: yes, the steps are in comment 12, additionally the Slack /remind command can be used to cause a delayed message and very the unread messages status [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: the patch is trivial and comes with a test [String changes made/needed]: none

Comment on attachment 8913296 [details] Bug 1402373 - Slack's red badge favicon is shown for no reason and then isn't cleared when all messages are read. Taking it. Experiencing this myself, this is really a pain (I thought it was a bug from Slack). Should be in 57b5

I suggest to wait the uplift for bug 1401777 before pushing this, to avoid bitrotting. I'm already testing the beta patch for that on Try, so it should be very soon.

Reproduced using comment 12 on Nightly 2017-09-22. Verified as fixed using Firefox 57 beta 13 and latest Nightly 58.0a1 2017-10-31 under Win 10 64-bit, Ubuntu 16.04 64-bit and Mac OS X 10.13.