"tabs" Module conflicts with Panorama: groups getting mangled for the last few nightly updates

RESOLVED FIXED in 1.0b3

Status

defect
P4
normal
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: dietrich, Assigned: Felipe)

Tracking

unspecified
1.0b3
x86
All
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
over the last few days, when i update to the nightly build, my tabcandy groups get messed up.

once, almost all tabs were put into one group. another time, the set of tabs in group B were all in group A, and an equivalent number from A were moved into B.

i made an add-on that acts on TabOpen events, so i need to confirm for sure that the addon isn't interacting badly with Panorama.
(Reporter)

Updated

8 years ago
Blocks: 627096
I was going to confirm this, since I've been seeing this for a couple of days too. But firstly I decided to check if it's not an extension's fault and it turned out it was happening because of the "Hard Blockers Counter 0.8" extension.

In case it matters, the behavior was similar to that described your comment:
After startup everything is fine, but after entering panorama all or most tabs ended up being in the first group. After disabling the addon, TWO browser restarts are needed to fix the problem.

Mozilla/5.0 (Windows NT 6.1; rv:2.0b10pre) Gecko/20110124 Firefox/4.0b10pre
P.S. the up to date "Hard Blockers Counter 1.0.1" does not cause this issue.
Sorry for bugspam, but actually even the latest version of the addon causes the issue...
can you post steps to reproduce?
There probably is shorter way to achieve this but that's what I did:

1. Start Minefield with a clean profile
2. Set Minefield to load previous session on startup
3. Install Hard Blockers Counter 1.0.1 from AMO
4. Have 4 tabs with something loaded
5. Restart Minefield (not sure if it's necessary)
6. Open Panorama
7. Create a new group and move one tab from the first group
8. Create one more group and move a tab from the first group
9. Select a tab from the first group
10. Restart Minefield
11. Notice that 2 tabs are shown in the tab bar
12. Open Panorama
13. Notice that 3 (sometimes 4) tabs are shown in the first group
14. Choose a tab from the first group
15. Notice that 3 or 4 tabs are shown on the tab bar

Seems to be reproducible always on my machine. Works OK without the addon.
reproduced.

Simplified steps:
1. Minefield with 4 tabs open and HBC extension
2. Create two new groups in panorama and move one tab to each
3. Restart browser
4. Enter Panorama mode

Current behavior:
One tab from one of the new groups is moved to the default group and becomes visible after step 4 when viewing default group.

What's interesting is that after I disable HBC and restart the browser I still can reproduce it until I restart the browser yet again. It seems like if the tabs were pinned to the wrong groups and I need to manually pin a tab to the right one with HBC disabled for it to stick.
I tested if the Widget itself causes it but it seems that it is not Widget module.

We should try to minimize it - I suspect that maybe widget+panel, timer or simpleStorage may be triggering it.

Comment 8

8 years ago
Should this bug be closed? Repurposed with a more specific summary?
I've been hitting this too, but glad that I can blame something now.

Gandalf: you used the addon-sdk for your extension, right? You have some suspicions here about what might be causing it, so I'm CC'ing Myk in case he has any ideas.

(In reply to comment #8)
> Should this bug be closed? Repurposed with a more specific summary?

Closed, no. Renamed/moved, maybe. It could be something from the addon-sdk misbehaving or an interaction that is somehow mishandled by panorama.
Priority: -- → P4
Summary: groups getting mangled for the last few nightly updates → Conflict with "Hard Blockers Counter" add-on: groups getting mangled for the last few nightly updates
wondering if this may be related to bug 625988 ...
(Reporter)

Comment 11

8 years ago
(In reply to comment #10)
> wondering if this may be related to bug 625988 ...

I never use PB, and my groups are remixed almost every time I restart the browser.
Dietrich: I didn't mean that PB causes it, but that there may be a common factor involved.
I wanted to minimize the testcase, but after an update to latest nightly on mac (4.0b11pre 2011-01-26) I cannot reproduce it anymore.

Can you confirm it still exists?
I was wrong in my previous comment - the bug still exists but it's sometimes hard to trigger it.

I was able to minimize my testcase in order to reproduce it. It was as simple as: 

main.js:
"let tabs = require("tabs")"

This line triggers the bug. The extension I used to test is stored here:

https://builder.addons.mozilla.org/addon/1000097/latest/

And I'll attach an .xpi with updated maxVersion to test against latest nightlies.
Posted file testcase .xpi
minimized extension that triggers this bug.

Note: When you're reproducing the bug it is sometimes required to remove all additional panorama groups, restart browser, add new groups, drag&drop tabs to reinitialize the bug.

Also, when you disable the extension it takes two cycles (restart, enter panorama, drag&drop back to the second group, restart ...) to stop triggering this bug.

That causes me to suspect that some code responsible for attaching tab to the given group doesn't work due to the Jetpack module "tabs" being initialized.
Duplicate of this bug: 628878
Duplicate of this bug: 629359
Summary: Conflict with "Hard Blockers Counter" add-on: groups getting mangled for the last few nightly updates → "tabs" Module conflicts with Panorama: groups getting mangled for the last few nightly updates
Duplicate of this bug: 627866
(Assignee)

Updated

8 years ago
Component: TabCandy → General
Product: Firefox → Add-on SDK
QA Contact: tabcandy → general
Version: Trunk → unspecified
(Assignee)

Updated

8 years ago
Assignee: nobody → felipc
OS: Linux → All
Blocks: 626527
I tried disabling the "Hard Blockers Counter" addon, which was the only recent addon I remember adding, but this is still killing me every time I restart the browser.
(Reporter)

Comment 20

8 years ago
(In reply to comment #19)
> I tried disabling the "Hard Blockers Counter" addon, which was the only recent
> addon I remember adding, but this is still killing me every time I restart the
> browser.

What other add-ons do you have?
(In reply to comment #19)
> I tried disabling the "Hard Blockers Counter" addon, which was the only recent
> addon I remember adding, but this is still killing me every time I restart the
> browser.

FWIW: This happened for me with *any* Jetpack / Add-on SDK based addon. It took disabling all addons and re-enabling one-by-one after restarts to narrow that down, though. So, not knowing what other add-ons you might have running, I'd say try that. It's a pain, but less pain than having the tab groups shuffle

Comment 23

8 years ago
1
(Reporter)

Comment 24

8 years ago
(In reply to comment #22)
> Pull request:
> https://github.com/mozilla/addon-sdk/pull/103

thanks for tracking this down Felipe. can you attach the patch here and ask adw for review?
(Assignee)

Comment 25

8 years ago
Posted patch PatchSplinter Review
Small change from the pull request, fixed an existing strict mode warning (missing "let" in for loop) and updated doc to clarify the close event behavior.

Grabbing text from the the pull request:
----
The tabs module conflicts with panorama because it manually calls tab.close() when a window is closed, which interferes with the browser itself closing them. With this change we'll just emit the onclose event instead, which was the original purpose of calling tab.close().

There was another problem in which only half of the tabs were receiving the onclose event due to elements being removed from the tabs list during an iteration on the list.
Tests included for both changes
Attachment #509548 - Flags: review?(adw)
Comment on attachment 509548 [details] [diff] [review]
Patch

I don't understand the tabs and windows code at all and am having a hard time trying to follow it.  r+'s for everyone!

This comment on line 116 of addon-kit/lib/windows.js isn't accurate anymore, so could you update or remove it?

  // Need to remove all the tabs before window listener are notified.
Attachment #509548 - Flags: review?(adw) → review+
(Assignee)

Comment 27

8 years ago
pull request for checkin, with the outdated comment removed.

https://github.com/mozilla/addon-sdk/pull/106
Thanks Felipe!

https://github.com/mozilla/addon-sdk/commit/2496d77232d02dd130c815b615eea9c8c8f2fb49

https://github.com/mozilla/addon-sdk/commit/7beedfce2986784887456c076ca0b8fe7b72d1d8
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0b3

Updated

8 years ago
Duplicate of this bug: 633849
Duplicate of this bug: 637323
Duplicate of this bug: 640180
Duplicate of this bug: 644055
Duplicate of this bug: 646390
Duplicate of this bug: 648602
You need to log in before you can comment on or make changes to this bug.