Opened 19 months ago

Closed 19 months ago

Last modified 19 months ago

#6511 closed defect (fixed)

Actions inside popup ui notifications do not properly remove notification

Reported by: Ross Assignee: kzar
Priority: P1 Milestone: Adblock-Plus-3.0.3-for-Chrome-Opera-Firefox
Module: Platform Keywords:
Cc: sebastian, kzar, greiner, saroyanm Blocked By: #6476, #6510, #6512
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29730611/

Description (last modified by kzar)

Environment

ABP 3.0.2.1983
Chrome 49 / 65 / Windows 7
Opera 49 / Windows 7

How to reproduce

  1. Save the notification below into a directory.
  2. Serve it python -m SimpleHTTPServer
  3. Install ABP.
  4. [ABP Icon] > [Right Click Popup] > [Inspect].
  5. In the popup console, set the notification url: browser.runtime.sendMessage({type: "prefs.set", key: "notificationurl", value: "<serveurl>"})
  6. Wait for ABP to request notifications.
  7. Open the popup UI to see notification.
  8. Select the [X] and [Close this notification].
  9. Open popup UI again.
  1. Select the [X] and [Stop showing notifications]

Test data

{
  "notifications": [
    {
      "id": "1",
      "message": {
        "en-US": "This is a information notification."
      },
      "severity": "information",
      "title": {
        "en-US": "Test Normal Notification"
      }
    }
  ],
  "version": "201707281538"
}

Observed behaviour

The [Close this notification] action closes the notification. But the notification reappears again when the popup ui is reopened.

The [Stop showing notifications] action seems to unset the [Show useful notifications] option in [Advanced]. But does not remove the notification from the popup ui as with [Close this notification] above.

Expected behaviour

In the 3.0.2 release, selecting [Close this notification] removes the notification from the UI and it is not present when the popup ui is reopened. The same with [Stop showing notifications].

What to change

  • Update the adblockplusui dependency to hg:9546f62e182f git:7411639 which will include the fixes for #6511 and #6510.
  • Send a "notifications.clicked" message instead of calling notification.onClicked() from the popup code.
  • Implement notificationHelpre.notificationClicked.

Notes

Hints for testers

  • Test that #6511 and #6510 are fixed.
  • No other changes are included, so nothing else should need to be tested.

Change History (12)

comment:1 Changed 19 months ago by kzar

  • Cc greiner saroyanm added
  • Owner set to kzar
  • Priority changed from Unknown to P1
  • Ready set

I can reproduce this, investigating now.

comment:2 Changed 19 months ago by kzar

  • Description modified (diff)
  • Priority changed from P1 to P2

comment:3 Changed 19 months ago by kzar

  • Description modified (diff)

comment:4 Changed 19 months ago by kzar

  • Blocked By 6512 added

comment:5 Changed 19 months ago by kzar

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:6 Changed 19 months ago by kzar

  • Priority changed from P2 to P1

Despite the problematic commit landing in October 2017 this is a regression since the last Chrome release. Putting it back to P1.

comment:7 Changed 19 months ago by sebastian

  • Blocked By 6476 added
  • Milestone set to Adblock-Plus-3.0.3-for-Chrome-Opera-Firefox

comment:8 Changed 19 months ago by sebastian

  • Blocked By 6510 added
  • Description modified (diff)

comment:9 Changed 19 months ago by kzar

  • Description modified (diff)

comment:10 Changed 19 months ago by abpbot

A commit referencing this issue has landed:
Issue 6511 - Use messaging to register popup notification clicks

comment:11 Changed 19 months ago by kzar

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:12 Changed 19 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Fixed.

ABP 3.0.2.1998
Chrome 49 / 65 / Windows 7

Last edited 19 months ago by Ross (previous) (diff)
Note: See TracTickets for help on using tickets.