Issue 5588 - Explicitly clear badge text on tab update

Issue 5588 - Explicitly clear badge text on tab update

Chrome automatically clears the browser action badge text when the URL of the tab is updated, but Firefox doesn't.

[Manish Jethani, 2017-08-29 20:49:03]

Patch Set 1

[Sebastian Noack, 2017-08-29 23:07:01]

https://codereview.adblockplus.org/29530598/diff/29530599/lib/stats.js
File lib/stats.js (right):

https://codereview.adblockplus.org/29530598/diff/29530599/lib/stats.js#newcode38
lib/stats.js:38: // the tab is updated, but Firefox doesn't.
Please refer to the Firefox bug here. If there isn't one yet, please file one.

[Wladimir Palant, 2017-08-30 08:13:30]

https://codereview.adblockplus.org/29530598/diff/29530599/lib/stats.js
File lib/stats.js (right):

https://codereview.adblockplus.org/29530598/diff/29530599/lib/stats.js#newcode38
lib/stats.js:38: // the tab is updated, but Firefox doesn't.
On 2017/08/29 23:07:01, Sebastian Noack wrote:
> Please refer to the Firefox bug here. If there isn't one yet, please file one.

I filed a bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1395074

[Manish Jethani, 2017-08-30 10:29:16]

On 2017/08/30 08:13:30, Wladimir Palant wrote:
> https://codereview.adblockplus.org/29530598/diff/29530599/lib/stats.js
> File lib/stats.js (right):
> 
>
https://codereview.adblockplus.org/29530598/diff/29530599/lib/stats.js#newcode38
> lib/stats.js:38: // the tab is updated, but Firefox doesn't.
> On 2017/08/29 23:07:01, Sebastian Noack wrote:
> > Please refer to the Firefox bug here. If there isn't one yet, please file
one.
> 
> I filed a bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1395074

Thanks.

They may not consider this to be a bug, by the way, since the documentation
states that the text is set for the given tab ID. The tab ID doesn't change upon
navigation, only the URL changes. Nevertheless, extension developers would
expect this to work like Chrome; they should at least mention what the standard
practice is to clear the text. I have assumed chrome.tabs.onUpdated is the place
to do it.

I also noticed that Firefox dispatches chrome.tabs.onUpdated twice on
navigation, once without changeInfo.url set and once with. It shouldn't matter
in this case, but something to keep in mind.

I'll update the comment with a link to the Firefox bug.

[Manish Jethani, 2017-08-30 10:30:35]

Patch Set 2: Add link to Firefox bug

https://codereview.adblockplus.org/29530598/diff/29530599/lib/stats.js
File lib/stats.js (right):

https://codereview.adblockplus.org/29530598/diff/29530599/lib/stats.js#newcode38
lib/stats.js:38: // the tab is updated, but Firefox doesn't.
On 2017/08/29 23:07:01, Sebastian Noack wrote:
> Please refer to the Firefox bug here. If there isn't one yet, please file one.

Done.

[Wladimir Palant, 2017-08-30 13:00:35]

LGTM

We'll need to make sure that this doesn't reset the badge after it already shows
data for the new page. I don't think that this should happen, but will keep an
eye on that just in case.

[Wladimir Palant, 2017-08-30 13:02:08]

On 2017/08/30 10:29:16, Manish Jethani wrote:
> They may not consider this to be a bug, by the way, since the documentation
> states that the text is set for the given tab ID.

It's a compatibility issue to be addressed nevertheless, and be it by updating
documentation. As long as there is a simple work-around, we don't have to care
how they address this.

[Sebastian Noack, 2017-08-30 16:56:28]

LGTM