Opened 4 years ago

Last modified 3 years ago

#4804 closed defect

Icon animations do not play properly for newly created tab — at Version 4

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 (4)

comment:1 Changed 4 years 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 4 years ago by kzar

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

comment:3 Changed 4 years 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 3 years ago by kzar

  • Review URL(s) modified (diff)
Note: See TracTickets for help on using tickets.