>+++ b/browser/base/content/tabview/ui.js ... >+ // Don't return to TabView if there are any app tabs >+ for (let a = 0; a < gBrowser.tabs.length; a++) { >+ let theTab = gBrowser.tabs[a]; >+ if (theTab.pinned && gBrowser._removingTabs.indexOf(theTab) == -1) >+ return; >+ } I don't think you need to iterate over the whole list of tabs... AFAIK AppTabs are always at the beginning, so you could break early. for (let a = 0; a < gBrowser.tabs.length; a++) { let theTab = gBrowser.tabs[a]; if (!theTab.pinned) break; if (gBrowser._removingTabs.indexOf(theTab) == -1) return; }

Not a big deal, but might as well take care of it. Raymond, would you mind taking this on as well?

Sent it to try and waiting for the results. http://tbpl.mozilla.org/?tree=MozillaTry&rev=146cac2a9075

Passed Try!

Comment on attachment 523957 [details] [diff] [review] v1 I personally don't really like the break and return statements combined - it's pretty hard to read (and get what it's doing). I'd prefer the following: >-for (let a = 0; a < gBrowser.tabs.length; a++) { >+for (let a = 0; a < gBrowser._numPinnedTabs; a++) { Would you concur with that solution? We could also ask Ian to throw in his two cents :)

Taking tim's suggestion.

Comment on attachment 524338 [details] [diff] [review] v2 Looks great! Thank you.

(In reply to comment #7) > Created attachment 524576 [details] [diff] [review] > Patch for checkin Passed Try! http://tbpl.mozilla.org/?tree=MozillaTry&rev=56785418a284