1570961 - The WNP impression ping is wrongly sent when the panel is triggered even if there are no messages to be displayed

Status: RESOLVED DUPLICATE of bug 1590721

(Firefox :: Messaging System, defect, P3)

Description

[Ciprian Muresan [:cmuresan], Formerly of Ecosystem QA - please needinfo @mcoman for things regarding the team]

Attachment: impression ping is wrongly sent when the button has no messages in it2.gif

[Affected versions]:

• Firefox Nightly 70.0a1, Build ID 20190802094835

[Affected Platforms]:

• All Windows
• All Mac
• All Linux

[Prerequisites]:

• Have the browser.newtabpage.activity-stream.asrouter.devtoolsEnabled pref set to true.
• Have the browser.ping-centre.log pref set to true.

[Steps to reproduce]:

1. Start the browser with a new clean profile and open the Browser Console.
2. Refocus the Browser and navigate to about:newtab#devtools.
3. Select the panel_local_testing option from the Show messages dropdown.
4. Click the Show button for the the WHATS_NEW_BADGE_70` option in the list.
5. Wait 5 seconds and observe the Browser Console.

[Expected results]:

• No new messages appear in the Browser Console.

[Actual results]:

• An impression ping is logged in the Browser Console even though the toolbar button is not displayed.

[Notes]:

• Not sure if this is strictly related to how I'm triggering the button.
• Attached a screen recording of the issue.

Comment 1

[Andrei Oprea [:andreio]]

I'm turning this into a P3.

1. It's not possible to happen given our current messages setup: you can't dismiss What's New messages so the panel will always have content. Similar for FxA
2. It's tricky to fix: the function that actually adds the badge will be called for every window. Assuming we only want to count 1 impression for all we need to do it one level above where unfortunately we can't tell if the operation was executed successfully. We need a function that calls addImpression once and returns early for subsequent calls.

Comment 2

[Kate Hudson :k88hudson]

Didn't you land something recently that fixed this? Can you link the bug/PR?