727930 - Windows that are getting opened are not added in the tab list popup while it is open

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

See testcase, steps to reproduce:
- make sure you have at least 2 tabs open.
- Tap on the "open windows 5 ones after 5 seconds" button.
- Tap on "Always Show" for the doorhanger prompt that appears after 5 seconds.
- Tap again on the "open windows 5 ones after 5 seconds" button.
- Open the tab list popup within 5 seconds.

Expected result:
- The windows that are getting opened are getting shown in the tab list popup

Actual result:
- None of the 5 windows that are getting opened are shown in the tab list popup.

Comment 1

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

This takes care of tabs added when tabs-tray is shown.
The code is refactored a bit for better clarity.
The index check inside position check sounds meaningless -- however, I saw some crashes without it. (Basically if index is -1, the tab is closed -- but still I get index = -1 exception error).

Comment 2

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 597975 [details] [diff] [review]
Patch

Found the bug in the patch. Posting a new patch soon.

Comment 3

[Sriram Ramasubramanian [:sriram]]

Attachment: Patch

The previous patch had missed a return statement. That has been fixed in this patch now. Everything works without a crash :D

Comment 4

[:Margaret Leibovic]

Comment on attachment 597978 [details] [diff] [review]
Patch

># HG changeset patch
># User Sriram Ramasubramanian <[e-mail]>
># Date 1329426872 28800
># Node ID d1a01df07f7ee3e508f5318ee9534b55e64badb5
># Parent  6989376471f7deb2579f360cce19d883130f2532
>Bug 727930: Tabs-tray should honour new tabs added in the background.
>
>diff --git a/mobile/android/base/Tabs.java b/mobile/android/base/Tabs.java
>--- a/mobile/android/base/Tabs.java
>+++ b/mobile/android/base/Tabs.java
>@@ -81,24 +81,25 @@ public class Tabs implements GeckoEventL
>         if (tabs.containsKey(id))
>            return tabs.get(id);
> 
>         String url = params.getString("uri");
>         Boolean external = params.getBoolean("external");
>         int parentId = params.getInt("parentId");
>         String title = params.getString("title");
> 
>-        Tab tab = new Tab(id, url, external, parentId, title);
>+        final Tab tab = new Tab(id, url, external, parentId, title);
>         tabs.put(id, tab);
>         order.add(tab);
> 
>         if (!mRestoringSession) {
>             GeckoApp.mAppContext.mMainHandler.post(new Runnable() {
>                 public void run() {
>                     GeckoApp.mBrowserToolbar.updateTabCountAndAnimate(getCount());
>+                    GeckoApp.mAppContext.onTabsChanged(tab);

Since the world has changed since this patch was written, you probably want to make this notifyListeners(tab, TabEvents.ADDED) and add ADDED to TabEvents. And then you can probably update TabsTray.onTabsChanged to act accordingly based on the message.

Comment 5

[Sriram Ramasubramanian [:sriram]]

That bug is on me :D and I thought of changing onTabsChanged() here once I get approval for this patch. It's on track :D

Comment 6

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Ping mfinkle for review.

Comment 7

[Sriram Ramasubramanian [:sriram]]

Might have been fixed by other bugs.