Closed
Bug 628188
Opened 14 years ago
Closed 14 years ago
"tabs" Module conflicts with Panorama: groups getting mangled for the last few nightly updates
Categories
(Add-on SDK Graveyard :: General, defect, P4)
Tracking
(Not tracked)
RESOLVED
FIXED
1.0b3
People
(Reporter: dietrich, Assigned: Felipe)
References
Details
Attachments
(2 files)
(deleted),
application/octet-stream
|
Details | |
(deleted),
patch
|
adw
:
review+
|
Details | Diff | Splinter Review |
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.
Comment 1•14 years ago
|
||
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
Comment 2•14 years ago
|
||
P.S. the up to date "Hard Blockers Counter 1.0.1" does not cause this issue.
Comment 3•14 years ago
|
||
Sorry for bugspam, but actually even the latest version of the addon causes the issue...
Comment 4•14 years ago
|
||
can you post steps to reproduce?
Comment 5•14 years ago
|
||
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.
Comment 6•14 years ago
|
||
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.
Comment 7•14 years ago
|
||
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•14 years ago
|
||
Should this bug be closed? Repurposed with a more specific summary?
Comment 9•14 years ago
|
||
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.
Updated•14 years ago
|
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
Comment 10•14 years ago
|
||
wondering if this may be related to bug 625988 ...
Reporter | ||
Comment 11•14 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.
Comment 12•14 years ago
|
||
Dietrich: I didn't mean that PB causes it, but that there may be a common factor involved.
Comment 13•14 years ago
|
||
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?
Comment 14•14 years ago
|
||
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.
Comment 15•14 years ago
|
||
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.
Updated•14 years ago
|
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
Assignee | ||
Updated•14 years ago
|
Component: TabCandy → General
Product: Firefox → Add-on SDK
QA Contact: tabcandy → general
Version: Trunk → unspecified
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → felipc
OS: Linux → All
Comment 19•14 years ago
|
||
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•14 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?
Comment 21•14 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.
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
Assignee | ||
Comment 22•14 years ago
|
||
Pull request:
https://github.com/mozilla/addon-sdk/pull/103
Comment 23•14 years ago
|
||
1
Reporter | ||
Comment 24•14 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•14 years ago
|
||
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 26•14 years ago
|
||
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•14 years ago
|
||
pull request for checkin, with the outdated comment removed.
https://github.com/mozilla/addon-sdk/pull/106
Comment 28•14 years ago
|
||
Thanks Felipe!
https://github.com/mozilla/addon-sdk/commit/2496d77232d02dd130c815b615eea9c8c8f2fb49
https://github.com/mozilla/addon-sdk/commit/7beedfce2986784887456c076ca0b8fe7b72d1d8
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0b3
You need to log in
before you can comment on or make changes to this bug.
Description
•