Last Comment Bug 625329 - Pinned tabs are used when setting homepage from current tabs
: Pinned tabs are used when setting homepage from current tabs
Status: RESOLVED FIXED
[fixed-in-fx-team]
:
Product: Firefox
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: All All
: -- normal with 3 votes (vote)
: Firefox 11
Assigned To: Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
:
: Jared Wein [:jaws] (please needinfo? me)
Mentors:
Depends on:
Blocks: 551849
  Show dependency treegraph
 
Reported: 2011-01-13 03:52 PST by Tolga Hosgor
Modified: 2011-12-14 06:41 PST (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v0.1 (4.85 KB, patch)
2011-08-11 15:50 PDT, Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
gavin.sharp: review-
Details | Diff | Splinter Review
Patch v0.2 (5.11 KB, patch)
2011-11-17 10:44 PST, Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
gavin.sharp: review-
Details | Diff | Splinter Review
Patch v0.3 (4.99 KB, patch)
2011-11-17 13:14 PST, Paul O'Shannessy [:zpao] (not reading much bugmail, email directly)
gavin.sharp: review+
Details | Diff | Splinter Review

Description Tolga Hosgor 2011-01-13 03:52:36 PST
User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8) Gecko/20100101 Firefox/4.0b8
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8) Gecko/20100101 Firefox/4.0b8

When you click "use opened tabs" for settings homepage in preferences menu, it counts application tab as normal tab and adds it too but when you restart the browser it opens 3 tabs, one app tab, one tab with same url as app tab, one tab with actual homepage.

Reproducible: Always

Steps to Reproduce:
1.make an app tab and normal tab
2.go pref. menu and click use current tabs to set homepage
3.save and restart
Actual Results:  
Application tab left acting the same way it does, I got two new tabs on startup.

Expected Results:  
Application tab should be a constant homepage-like URL on startup instead of the page I was at before closing Firefox.
Comment 1 Luke Iliffe (Harlequin99) 2011-01-13 07:42:49 PST
Confirmed on trunk.
Comment 2 Marco Bonardo [::mak] 2011-01-18 06:25:39 PST
setHomePageToCurrent does not seem to be aware of tabs and visibleTabs and so on, so it's also probably misbehaving with Panorama.
Probably it should use visibleTabs and ignore apptabs, adding uiwanted to define this.
Comment 3 Maniac Vlad Florin (:vladmaniac) 2011-01-19 06:14:38 PST
Confirm this on 

Mozilla/5.0 (Windows NT 6.1; rv:2.0b10pre) Gecko/20110118 Firefox/4.0b10pre
Comment 4 Maniac Vlad Florin (:vladmaniac) 2011-01-19 06:42:36 PST
This is happening so far as Fx4b6 candidate builds | all platforms
Comment 5 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-08-11 13:59:51 PDT
Bug 636149 made it so only visible tabs are used, but pinned tabs are still included.
Comment 6 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-08-11 15:50:44 PDT
Created attachment 552529 [details] [diff] [review]
Patch v0.1
Comment 7 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-08-11 16:04:45 PDT
Comment on attachment 552529 [details] [diff] [review]
Patch v0.1

>diff --git a/browser/components/preferences/main.js b/browser/components/preferences/main.js

>+    let _this = this;
>+    window.addEventListener("focus", function() _this._updateUseCurrentButton(), false);

Use .bind()?

>+  _getTabsForHomePage: function ()

>-    if (win && win.document.documentElement
>-                  .getAttribute("windowtype") == "navigator:browser") {

Isn't this check still needed to catch the cases where .opener is not a browser window? The previous logic didn't needed it for the command case since the button was disabled then.
Comment 8 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-11-17 10:44:26 PST
Created attachment 575227 [details] [diff] [review]
Patch v0.2

Addressed comments.
Comment 9 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-11-17 11:52:34 PST
Comment on attachment 575227 [details] [diff] [review]
Patch v0.2

Can't lost the window.opener null check in _getTabsForHomePage, since that isn't guaranteed to be non-null.
Comment 10 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-11-17 13:14:27 PST
Created attachment 575277 [details] [diff] [review]
Patch v0.3

Ah, I was being a bit naive there and assumed the only situation left was where the pref window is opened from another window.
Comment 11 Paul O'Shannessy [:zpao] (not reading much bugmail, email directly) 2011-12-13 12:20:39 PST
https://hg.mozilla.org/integration/fx-team/rev/b30dc8e42907
Comment 12 Rob Campbell [:rc] (:robcee) 2011-12-14 06:41:57 PST
https://hg.mozilla.org/mozilla-central/rev/b30dc8e42907

Note You need to log in before you can comment on or make changes to this bug.