Opened 2 months ago

Closed 4 weeks ago

Last modified 2 weeks ago

#4804 closed defect (fixed)

Icon animations do not play properly for newly created tab

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

https://codereview.adblockplus.org/29362605/

Description

Environment

Chrome 55
Adblock Plus 1.12.4

How to reproduce

  1. Install / reload Adblock Plus
  2. After Adblock Plus has loaded open a new tab
  3. Type this in the extension's background console: require("icon").startIconAnimation("critical");.

Observed behaviour

The icon for the newly created tab does not change. If you switch to a different tab, then switch back to the newly created tab during the animation the icon gets stuck for the new tab.

Expected behaviour

The animation plays properly, even for the newly created tab.

Notes

This is a regression caused by Issue 4386 - Fixed determining document domain, particularly after being redirected. (See also #4598, #4599 and #4647.)

Change History (6)

comment:1 Changed 2 months ago by kzar

It seems that this is caused by the fact that chrome.tabs.onUpdated is dispatched with the "loading" event before chrome.webNavigation.onCommitted.

Our chrome.tabs.onUpdated listener dispatches the ext.onLoading event, our listeners for that event add the new Page to their PageMaps. Shortly afterwards our chrome.webNavigation.onCommitted listener calls ext._removeFromAllPageMaps and the records of the page are trashed!

comment:2 Changed 2 months ago by kzar

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

comment:3 Changed 2 months ago by kzar

  • Priority changed from P3 to P2

(Increasing to P2 since this bug causes our PageMaps to be wrong half the time, which could well be causing other issues. Perhaps we should even mark this P1?)

comment:4 Changed 5 weeks ago by kzar

  • Review URL(s) modified (diff)

comment:5 Changed 4 weeks ago by kzar

  • Milestone set to Adblock-Plus-for-Chrome-Opera-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

A commit referencing this issue has landed:
Issue 4598,4599,4647,4804 - Work around onCommitted flaw

comment:6 Changed 2 weeks ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Looks fixed. Icon animations continue to play correctly when started on different tabs and after opening/closing tabs as described.

ABP 1.12.4.1741
Firefox 52 / Windows 10

ABP 1.12.4.1739
Chrome 49 / 56 / Windows 10
Chrome 56 / OS X 10.12
Chrome 56 / Ubuntu 16.04
Opera 37 / 41 / Windows 7
Safari 10 / OS X 10.12

Note: See TracTickets for help on using tickets.