1413676 - PageActions do not work when restoring lazy tabs

Status: RESOLVED DUPLICATE of bug 1413574

(WebExtensions :: Frontend, defect, P1)

Description

[Shane Caraveo (:mixedpuppy)]

Either they are not enabled (but the icon seems enabled) or the click handling is not right.  After loading an extension in about:debugging then switching to a tab with a webpage, the icon is visible, but does not indicate when I hover over it, and does not open when I click.  Initially it seemed to always be the *first* page I would switch to, but sometimes it seems I have to switch tabs a couple times.  Sometimes it seems like certain pages, but after switching tabs a few times, they start working.  Sometimes it just always works.

The extension just has a simple page action and a background script to enable it.

background.js:
browser.tabs.onActivated.addListener(details => {
  browser.pageAction.show(details.tabId);
});

Comment 1

[Shane Caraveo (:mixedpuppy)]

Oh, regarding the hover indication mentioned above.  The hover indication does in fact indicate whether it will work or not.  If you don't see it, click will do nothing.

Comment 2

[Drew Willcoxon :adw]

It sounds like the disabled/show state is messed up.  What you're describing -- not being able to click, hover not showing up -- is what happens for disabled actions, except the icons of disabled actions should be grayed out a little too.  I'll take a look.

Comment 3

[Drew Willcoxon :adw]

(In reply to Drew Willcoxon :adw from comment #2)
> except the icons of disabled actions should be grayed out a little too.

Ah but that only works with svg icons in Mozilla-signed extensions.  So there's actually no indication at all for other icons, which is a big problem... right?

Comment 4

[Drew Willcoxon :adw]

I can reproduce this every time with:

1. Start Firefox
2. Open about:debugging
3. Click Load Temporary Add-on, choose my test add-on with a page action and background script as in comment 0
4. Open a new tab
5. Switch back to the about:debugging tab
6. At this point, my action is enabled
7. Reload the page
8. The action is now disabled

Maybe there are other ways to do it too, or code paths other than what I'm hitting here where this problem happens.

What I'm seeing happen is, after step 5, the action's show() method gets called (due to the background script I guess), and the action is correctly enabled.  After step 7, handleLocationChange gets called and fromBrowse is true, so the tab data is cleared, and when updateButton is then called, `show` is not in tabData, so the action is disabled.

Adding this to my background script fixes it:

browser.tabs.onUpdated.addListener(tabId => {
  browser.pageAction.show(tabId);
});

If this is what's happening in other STRs too, then I wonder if this was just always broken, but it was harder to tell because we used to not show non-shown actions, but now we are.

It would be good to get other reliable STR.  I can also try my STR in a Firefox before bug 1395387 landed to see what happens.

Comment 5

[Drew Willcoxon :adw]

(In reply to Drew Willcoxon :adw from comment #4)
> I can also try my STR in a Firefox before bug 1395387 landed to see what happens.

The icon disappears, which would indicate that the problem of the action seeming to not work sometimes but work other times predates Photon page actions (bug 1395387).  But it would still be good to get some other reliable STR.

Comment 6

[Drew Willcoxon :adw]

More STR:

1. New profile
2. Open preferences, choose "show your windows and tabs from last time"
3. Install a simple page action extension
4. Open a normal web page, verify that the page action appears
5. Restart Firefox

On restart, the web page from step 4 should be the current tab, but the page action is disabled.  To enable it, switch to another tab and then switch back.

When I try this with a Nightly before bug 1395387 landed, the page action simply doesn't appear until after switching to another tab and then switching back.  So that's more evidence of what I saw in comment 5.

So to summarize, there are two problems here:

(1) With the simple background script in comment 0, the action is disabled (after bug 1395387) or hidden (before that bug) sometimes when it seems like it shouldn't be.  I don't know whether this is really unexpected/incorrect behavior.  Is the webextension API supposed to work like that?  But after bug 1395387 landed, it made the problem more obvious.  When I add a browser.tabs.onUpdated listener as in comment 4, this problem goes away.

(2) Disabled and enabled page actions look the same until you hover over them or try to click them, for non-Mozilla-signed extensions.  We either need to somehow dim the icons from these third-party extensions, or we go back to hiding non-shown actions (instead of disabling them), like bug 1413574 suggests.

Comment 7

[Shane Caraveo (:mixedpuppy)]

So I updated the background script to this:

browser.tabs.onActivated.addListener(details => {
  browser.pageAction.show(details.tabId);
});

browser.tabs.query({}).then(tabs => {
  for (let tab of tabs) {
    browser.pageAction.show(tab.tabId);
  }
})

Now after installing via about:debugging, the icon shows on existing tabs when I switch to them.  What I have noticed is that if I have a number of tabs open, when switching to a lazy tab, the icon remains disabled.

STR
- start firefox, open a couple/few tabs to various urls
- Open preferences, choose "show your windows and tabs from last time"
- restart firefox
- take note of which tab you are on
- open a new tab to about:debugging
- in about:debugging open a simple pageaction ext with above background script
- switch to the the tab that was selected upon restart
* the page action should be visible and workgin
- switch to any other tab that was not selected upon restart
* the page action should be visible, it does not work
- switch to another tab, then back to the last one
* the page action is visible and now correctly works

It seems to me that page action is broken if the tab was in a lazy state.

I'm also of the opinion that this predates bug 1395387.

I'm also now of the opinion that we should also "fix" bug 1413574

Comment 8

[Shane Caraveo (:mixedpuppy)]

This is actually not a result of the PageActions work, it's been around much longer.  I'm not certain if it's a regression at some earlier stage, or just an edge case we never caught before, so removing "regression".

Comment 9

[Shane Caraveo (:mixedpuppy)]

I can no longer recreate this problem via the STR what worked for me.  I'm taking a guess that this was a side effect caused by bug 1395387 and fixed by bug 1413574.  Something to keep in mind if/when the pageaction icons become always visible.