Closed Bug 625329 Opened 9 years ago Closed 8 years ago

Pinned tabs are used when setting homepage from current tabs

Categories

(Firefox :: Preferences, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 11

People

(Reporter: fasdfasdas, Assigned: zpao)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fixed-in-fx-team])

Attachments

(1 file, 2 obsolete files)

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.
Confirmed on trunk.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hardware: x86_64 → x86
Version: unspecified → Trunk
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.
Blocks: pinnedtabs
Keywords: uiwanted
Confirm this on 

Mozilla/5.0 (Windows NT 6.1; rv:2.0b10pre) Gecko/20110118 Firefox/4.0b10pre
This is happening so far as Fx4b6 candidate builds | all platforms
Bug 636149 made it so only visible tabs are used, but pinned tabs are still included.
Attached patch Patch v0.1 (obsolete) — Splinter Review
Assignee: nobody → paul
Attachment #552529 - Flags: review?(gavin.sharp)
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.
Attachment #552529 - Flags: review?(gavin.sharp) → review-
Keywords: uiwanted
OS: Windows 7 → All
Hardware: x86 → All
Summary: application tab is considered as normal while settings homepage → Pinned tabs are used when setting homepage from current tabs
Attached patch Patch v0.2 (obsolete) — Splinter Review
Addressed comments.
Attachment #552529 - Attachment is obsolete: true
Attachment #575227 - Flags: review?(gavin.sharp)
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.
Attachment #575227 - Flags: review?(gavin.sharp) → review-
Attached patch Patch v0.3Splinter Review
Ah, I was being a bit naive there and assumed the only situation left was where the pref window is opened from another window.
Attachment #575227 - Attachment is obsolete: true
Attachment #575277 - Flags: review?(gavin.sharp)
Attachment #575277 - Flags: review?(gavin.sharp) → review+
https://hg.mozilla.org/mozilla-central/rev/b30dc8e42907
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 11
See Also: → 1552808
Depends on: 1552808
You need to log in before you can comment on or make changes to this bug.