Closed Bug 707015 Opened 11 years ago Closed 11 years ago

BrowserApp.addTab(aURI) doesn't put the new tab in the foreground


(Firefox for Android Graveyard :: General, defect)

Not set


(Not tracked)



(Reporter: fabrice, Assigned: mfinkle)



(1 file)

That creates a new tab, and the title and favicon are changed, but not the web content.
An additional call to BrowserApp.selectTab() is needed.
Attached patch patchSplinter Review
This patch was a little more complicated than I thought it would be.

As fabrice menitions, this does not make the new tab active/selected): BrowserApp.addTab(uri, { selected: true });

You need to also call:

This is because the GeckoApp handleAddTab was only doing a tiny bit of selecting the new tab. It only told the Tabs object that the tab was selected. No other code was informed. Sending a Tab:Selected message, called handleSelectTab and that did a bunch of selection changes.

So, I made the Tab:Add code also call handleSelectTab if needed. In doing so, I could remove the "selected" param in handleAddTab, and let handleSelectTab do all the work. handleSelectTab needed a few checks added to it, since if this was the very first tab, no tab was already selected, so "selTab" could be null.

I also made some changes to browser.js because we had to tell Gecko that the new tab was active/selected too. Some minor cleanup is also in the code. Since BrowserApp.addTab defaults to making the tab active, we could remove some code that was already adding a separate call.

I tested about:home behavior, adding tabs via the UI and adding tabs via the code - all seem to work OK.
Assignee: nobody → mark.finkle
Attachment #578509 - Flags: review?(sriram)
Comment on attachment 578509 [details] [diff] [review]

This patch looks good to me.
Attachment #578509 - Flags: review?(sriram) → review+
would bug 706860 be a dup then since this looks like the underlying cause?
Do we still need things like:

It doesn't look like you're setting BrowserApp.selectedTab either. I don't really know this code. Is that trickling back from Java?
(In reply to Wesley Johnston (:wesj) from comment #4)
> Do we still need things like:
> content/browser.js#899
> It doesn't look like you're setting BrowserApp.selectedTab either. I don't
> really know this code. Is that trickling back from Java?

Thanks for taking a look:

BrowserApp.selectTab(...) sends a Tab:Selected" message back to Java. We don't want that. We just want the tab to be selected in browser.js because the "Tab:Added" handler will do call handleSelectTab too, if needed.

| = true | actually selects and activates a tab in browser.js, which we do in BrowserApp.addTab(...) if needed.

We should have your concerns covered here.
Closed: 11 years ago
Resolution: --- → FIXED
I wasn't able to reproduce this issue on the latest native fennec build:
Build ID: Mozilla/5.0 (Android; Linux armv7l; rv:11.0a1) Gecko/20111206 Firefox/11.0a1 Fennec/11.0a1 - Native Fennec build
Device: Samsung GalaxyS, Android 2.2
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.