Skip to content
This repository was archived by the owner on Feb 29, 2020. It is now read-only.

Conversation

@piatra
Copy link
Contributor

@piatra piatra commented May 29, 2019

…e CFR Pin Tab visits counter

Note: This is only required for the pinned tabs trigger. For addon recommendations if you open a tab in the background you won't get a notification and no impression is counted.

TODO:

  • mochitest for this case

Copy link
Contributor

@k88hudson k88hudson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still something we want to land?

@piatra
Copy link
Contributor Author

piatra commented Sep 12, 2019

The issue still happens:

  • If you right-click open in a new tab or CMD/CTRL click to open in a new tab this triggers the onLocationChange listener (even if you haven't switched to this tab yet).
  • It's not obvious to me why this happens: I would interpret onLocationChange as firing only when the location of the focused browser changes and this is the behavior implemented in the trigger listener

But it's not that important we could also close this.

*/
function isFocusedTab(aBrowser, aLocationURI) {
try {
if (aBrowser.ownerGlobal.gBrowser.currentURI.spec === aLocationURI.spec) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure you can do aBrowser === aBrowser.ownerGlobal.gBrowser.selectedBrowser and not sure if you need a try/catch for nsIURI stuff anymore

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants