Opened 6 months ago

Closed 6 months ago

Last modified 4 weeks ago

#7340 closed defect (fixed)

Notification animations do not work

Reported by: Ross Assignee: hfiguiere
Priority: P1 Milestone: Adblock-Plus-3.5-for-Chrome-Opera-Firefox
Module: Platform Keywords:
Cc: sebastian, kzar, manish Blocked By:
Blocking: Platform: Chrome
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/30025579

Description

Environment

ABP 3.4.3.2260
Chrome 72.0.3626.121 / Windows 10

Regression on Chrome. Not able to check on FF right now.

How to reproduce

  1. Serve notification file below.
  2. Install ABP.
  3. Inspect the ABP popup, in the console run:

browser.runtime.sendMessage({type: "prefs.set", key: "notificationurl", value: "http://urlto.the/notification.json"})

  1. Wait 60s until you see notification has been requested.

Test notification

{
  "notifications": [
    {
      "id": "information_onelink",
      "severity": "information",
      "links": [
        "adblock_browser_promotion_0"
      ],
      "title": {
        "en-US": "Test Information Notification",
        "de": "DE Test Information Notification"
      },
      "message": {
        "en-US": "This is a <a>test</a> information notification.",
        "de": "DE This is a <a>test</a> information notification."
      }
    }
  ],
  "version": "201810011538"
}

Observed behaviour

The follow error appears in the console, while attempting to animate the notification:

adblockplus.js:11796 Uncaught TypeError: browser.browserAction.setIconPath is not a function
    at setIcon (adblockplus.js:11796)
    at setInterval (adblockplus.js:11889)

Expected behaviour

The error not to be logged to the console and icon animations to work as expected. The information notification should animate a ?, the critical notification should animate a !.

Change History (9)

comment:1 Changed 6 months ago by hfiguiere

  • Owner set to hfiguiere

comment:2 Changed 6 months ago by hfiguiere

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

comment:3 Changed 6 months ago by kzar

  • Milestone set to Adblock-Plus-3.5-for-Chrome-Opera-Firefox
  • Priority changed from Unknown to P1
  • Ready set

comment:4 Changed 6 months ago by sebastian

  • Cc sebastian added; snoack removed

comment:5 Changed 6 months ago by abpbot

A commit referencing this issue has landed:
Issue 7340 - Fix JS error with notification animation

comment:6 Changed 6 months ago by abpbot

A commit referencing this issue has landed:
Issue 7340 - Fix JS error with notification animation

comment:7 Changed 6 months ago by hfiguiere

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

comment:8 Changed 6 months ago by Ross

Fixed in Chrome/Opera. It is actually a problem on Edge too, however that is not a regression (#7347).

ABP 3.4.3.2261
Chrome 72.0.3626.121 / Windows 10
Opera 58.0.3135.79 / Windows 10

comment:9 Changed 4 weeks ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Fixed. Notification icons are working where expected.

ABP 0.9.15.2339
Microsoft Edge 44.17763.1.0 / Windows 10 1809

ABP 3.5.2.2340
Chrome 49.0.2623.75 / Windows 10 1809
Chrome 75.0.3770.142 / Windows 10 1809
Opera 36.0.2130.65 / Windows 10 1809
Opera 62.0.3331.72 / Windows 10 1809
Firefox 51.0 / Windows 10 1809
Firefox 68.0 / Windows 10 1809
Firefox Mobile 68.0 / Android 7.2.2

Note: See TracTickets for help on using tickets.