Closed
Bug 670684
Opened 13 years ago
Closed 11 years ago
Remove all tabs panel code
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 21
People
(Reporter: dao, Assigned: dao)
References
Details
Attachments
(1 file, 11 obsolete files)
56.45 KB,
patch
|
Details | Diff | Splinter Review |
I see no chance for this to get enabled anymore, so we might as well remove it.
Attachment #545191 -
Flags: review?(gavin.sharp)
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → dao
Assignee | ||
Comment 1•13 years ago
|
||
missed firefox.js
Attachment #545191 -
Attachment is obsolete: true
Attachment #545518 -
Flags: review?(gavin.sharp)
Attachment #545191 -
Flags: review?(gavin.sharp)
Comment 2•13 years ago
|
||
Comment on attachment 545518 [details] [diff] [review] patch >diff --git a/browser/base/content/browser-tabPreviews.js b/browser/base/content/browser-tabPreviews.js >+ document.getElementById("menu_showAllTabs").hidden = >+ !enable || !allTabs.canOpen; > > // Also disable the <key> to ensure Shift+Ctrl+Tab never triggers > // Show All Tabs. > var key_showAllTabs = document.getElementById("key_showAllTabs"); > if (enable) Don't these conditions need to match? I found a few references that seem to have been missed: ./base/content/browser.css - various allTabs-* references ./themes/winstripe/browser/browser-aero.css: #allTabs-panel KUI-panel-closebutton references
Attachment #545518 -
Flags: review?(gavin.sharp) → review-
Assignee | ||
Comment 3•13 years ago
|
||
(In reply to comment #2) > Comment on attachment 545518 [details] [diff] [review] [review] > patch > > >diff --git a/browser/base/content/browser-tabPreviews.js b/browser/base/content/browser-tabPreviews.js > > >+ document.getElementById("menu_showAllTabs").hidden = > >+ !enable || !allTabs.canOpen; > > > > // Also disable the <key> to ensure Shift+Ctrl+Tab never triggers > > // Show All Tabs. > > var key_showAllTabs = document.getElementById("key_showAllTabs"); > > if (enable) > > Don't these conditions need to match? They don't need to match. The key handler will just do nothing if and when the button isn't around.
Attachment #545518 -
Attachment is obsolete: true
Attachment #548136 -
Flags: review?(gavin.sharp)
Comment 5•12 years ago
|
||
Comment on attachment 548136 [details] [diff] [review] patch v2 It looks like this needs to be updated to account for bug 714281 (allTabs.canOpen won't return the correct value when the button is hidden) and bug 701212. I'll review quickly after that...
Attachment #548136 -
Flags: review?(gavin.sharp)
Assignee | ||
Updated•12 years ago
|
Assignee | ||
Comment 6•12 years ago
|
||
Attachment #548136 -
Attachment is obsolete: true
Attachment #618769 -
Flags: review?(gavin.sharp)
![]() |
||
Comment 7•12 years ago
|
||
Comment on attachment 618769 [details] [diff] [review] patch v3 Review of attachment 618769 [details] [diff] [review]: ----------------------------------------------------------------- Here's an unbitrotted diff: http://people.mozilla.org/~fyan/tmp/removealltabs.diff
Attachment #618769 -
Flags: feedback+
Please, don't land this! My workflow heavily depends on this feature, and several people I know and told about it, all think it's great. I know it's useless code in the Firefox tree if it isn't used, but this feature just hasn't been given a chance. I have made a similar comment in bug 515095 comment 9 about browser.ctrlTab.previews, which should be enabled by default in my opinion. This one should, too! In that comment, I have already mentioned one reason why browser.allTabs.previews should be enabled (the search box), and one why it wouldn't harm even for the people who don't use it. I would like to add: - The added screenshot really helps to remember which page is inside the tab. - You don't have to scroll between all open tabs As far as I know, the only reason to remove it, is "It's not enabled by default, so dead code for most of the users". Especially in combination with browser.ctrlTab.previews, it's a killer feature. As I asked in the other bug: please reconsider! :)
Assignee | ||
Comment 9•11 years ago
|
||
Attachment #618769 -
Attachment is obsolete: true
Attachment #618769 -
Flags: review?(gavin.sharp)
Attachment #659268 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 10•11 years ago
|
||
Now showing/hiding menu_showAllTabs in a popupshowing event listener, since allTabs.canOpen isn't static.
Attachment #659268 -
Attachment is obsolete: true
Attachment #659268 -
Flags: review?(gavin.sharp)
Attachment #659270 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 11•11 years ago
|
||
Attachment #659270 -
Attachment is obsolete: true
Attachment #659270 -
Flags: review?(gavin.sharp)
Attachment #660000 -
Flags: review?(gavin.sharp)
![]() |
||
Comment 12•11 years ago
|
||
I’ve been using Dão’s ctrl-tab extension, which provides a panel and the MRU order, and just found that I have an extension installed for the features, both features are in Firefox, and one of the features is disabled by default. I’ve disabled the extension and enabled browser.ctrlTab.previews, but if this bug is fixed, I’ll have to reverse that?
Comment 13•11 years ago
|
||
Beyond context differences, the only significant change is an additional removal of "[type=menu]" in pinstripe's browser.css due to HiDPI landing.
Attachment #660000 -
Attachment is obsolete: true
Attachment #660000 -
Flags: review?(gavin.sharp)
Comment 14•11 years ago
|
||
Comment on attachment 695818 [details] [diff] [review] patch v4, updated to tip I only found one additional change needed: KUI-close.png is now unused and can be removed.
Attachment #695818 -
Flags: review+
Comment 15•11 years ago
|
||
This should be ready to land, I think. https://tbpl.mozilla.org/?tree=Try&rev=ddeec16b17a2
Attachment #695830 -
Flags: review+
Comment 16•11 years ago
|
||
Some changes are needed to browser/base/content/test/browser_ctrlTab.js.
![]() |
||
Comment 17•11 years ago
|
||
Attachment #695818 -
Attachment is obsolete: true
Attachment #695830 -
Attachment is obsolete: true
![]() |
||
Comment 18•11 years ago
|
||
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #16) > Some changes are needed to browser/base/content/test/browser_ctrlTab.js. While trying to fix browser_ctrlTab.js, I discovered that the ctrl+tab panel is broken in OS X on Retina Display Macs (HiDPI mode). (It gets stuck after the first press of [tab].) While I verified the reproducibility of this with :felipe and :dolske, they suggested that I get rid of the ctrl tabs panel altogether, since it is a non-default piece of UI that is unlikely to become the default. tl;dr of below paragraph: I actually like ctrl+tab MRU but only if tab strip itself is also MRU, in which ctrl+tab would both be MRU and intuitively linear, but that idea is a bit too radical for Firefox desktop perhaps. My thinking around tabbed browsing has evolved over the years, and I actually support the idea of ctrl+tab switching to the most recently selected tab, but only if the tab strip itself reflected this, i.e. the tab strip always kept itself arranged in most recently used order. This would match the ordering of the primary app switchers in Windows 8, iOS, and Android. The difference is that in those platforms, the app switcher (analogous to the tab strip) is not always visible. The tab strip being always visible makes this more problematic, and Firefox desktop is probably not the best place for this kind of experimentation.
Attachment #712320 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 19•11 years ago
|
||
Comment on attachment 712320 [details] [diff] [review] part 2 to remove ctrl tab code (depends on patch v5) Please don't hijack my bug :/
Attachment #712320 -
Attachment is obsolete: true
Attachment #712320 -
Flags: review?(gavin.sharp)
![]() |
||
Comment 20•11 years ago
|
||
Sorry.
Assignee | ||
Comment 21•11 years ago
|
||
Thanks for updating the patch. I got tired of it after having it kept up to date for a year or so.
Attachment #712318 -
Attachment is obsolete: true
Assignee | ||
Comment 22•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/729fe49f9e1f
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Comment 23•11 years ago
|
||
Now Panorama is getting removed, there is absolutely NO alternative in Firefox to get a visual overview of tabs (not even a less good one). Also, it might not have been enabled by default, but there are sites that wrote articles about it (e.g. http://www.ghacks.net/2009/11/04/tab-preview-features-in-windows-7/, http://www.favbrowser.com/how-to-enable-firefox-3-6-tab-preview/, http://lifehacker.com/5813650/enable-thumbnails-in-firefoxs-list-all-tabs-menu and more), so I'm sure it is better known than you think.
![]() |
||
Comment 24•11 years ago
|
||
This is not the same thing as Panorama.
Comment 25•11 years ago
|
||
I know. Panorama will be removed in bug 836758. I love browser.allTabs.previews for having a nice, graphical overview of my tabs with a search box. I was saying Panorama *could* do this too (although a hell of a lot slower), but without even that alternative, it'll be even worse for me that I lost this functionality. In the end, browser.allTabs.previews is what I really want...
Comment 26•11 years ago
|
||
Seriously? It was great feature, and IMO, just as Tim said, it should be enabled by default. Even if it's not default, it still should be available for more advanced users. That's what was great in Firefox: customizability. Please, stop making Firefox a simple little browser for older people and little children. *downgrading*
Comment 27•11 years ago
|
||
I agree... this is a great feature! Keep it.. Landis. (In reply to Tim from comment #8) > Please, don't land this! My workflow heavily depends on this feature, and > several people I know and told about it, all think it's great. I know it's > useless code in the Firefox tree if it isn't used, but this feature just > hasn't been given a chance. > > I have made a similar comment in bug 515095 comment 9 about > browser.ctrlTab.previews, which should be enabled by default in my opinion. > This one should, too! > > In that comment, I have already mentioned one reason why > browser.allTabs.previews should be enabled (the search box), and one why it > wouldn't harm even for the people who don't use it. I would like to add: > > - The added screenshot really helps to remember which page is inside the tab. > > - You don't have to scroll between all open tabs > > As far as I know, the only reason to remove it, is "It's not enabled by > default, so dead code for most of the users". > > Especially in combination with browser.ctrlTab.previews, it's a killer > feature. As I asked in the other bug: please reconsider! :)
Comment 28•11 years ago
|
||
(In reply to piotr.kunicki from comment #26) > Seriously? It was great feature, and IMO, just as Tim said, it should be > enabled by default. > Even if it's not default, it still should be available for more advanced > users. That's what was great in Firefox: customizability. > Please, stop making Firefox a simple little browser for older people and > little children. > *downgrading* AGREE! Keep Tab Thumbs. Landis.
Comment 29•11 years ago
|
||
Really ?! ... This was so **** useful .... Now I have to search for an addon that is doing the same (if it exists)....
Comment 30•11 years ago
|
||
Using FF 21. browser.allTabs.previews;true does not work any more. Really this was removed? I used it a lot. I'm 50 years old man and I love FF. I hate the lack of configuration that G chrome has. Is FF losing its features? The world already has G Chrome (or MSIE), does not need other. The world already has FF, does not want to loose it.
Comment 31•11 years ago
|
||
I too would like this feature back, I agree that panorama was very poor, but this was simple and effective. I understand the need to clean up a code base but please pass the code on to others so they can create an add on for those of us who want it back. Just like the plan for panorama. Or point us to the replacement plan... Just secretly killing features isn't good enough for mozilla.
Assignee | ||
Comment 32•11 years ago
|
||
(In reply to berryman.michael from comment #31) > I understand the need to clean up a code base but please pass the code on to > others so they can create an add on for those of us who want it back. Just > like the plan for panorama. All the removed code is available in the attached patch. Feel free to take it.
Comment 33•11 years ago
|
||
http://www.ghacks.net/2013/05/24/restore-firefoxs-all-tabs-preview-feature/
Comment 34•11 years ago
|
||
This is bad, this was the best feature in firefox. I have more than 200 tabs at once. Since firefox only loads the tab when clicked . I crash my firefox and restore the session back. With this i have 200 tabs but only when i open something using search field from all tabs preview, it will get loaded, does not use any CPU or memory . Chrome will reload every tab when you do a session restore and you know what happens when you restore more than 50 tabs on a midrange laptop. This was the only feature that made me to stick with Firefox, Now i think mozilla wants me to move to chrome. I tried the addon but it does not work. It only generates preview when i click the tab which is bad. This is the best feature compared to tab groups which i think no one uses.
Comment 35•11 years ago
|
||
browser.allTabs.previews=true was one of the best features! please return it back!
Comment 36•11 years ago
|
||
Hi, This is one of the best features - I cannot live without it and also this is one of the two reasons why I;m not on Chrome. The other is TabMixPlus. I have just downloaded version 20 and will downgrade. Please return the best firefox feature - I do not want Chrome :P
Comment 37•11 years ago
|
||
Is Mozilla copying the Gnome project roadmap: removing usefull advanced functionnalities without user agreement just because there are not broadly used? It's not an evolution, it's a regression. Yes, you can make a plugin to retrieve it, but it's not the easy way. There's in fact only two choices: either you listen to users and restore this, either nobody in the core team wants to care about this piece of code and you definitely remove it.
Comment 38•11 years ago
|
||
(In reply to visar.zejnullahu from comment #35) +1 A little for me and especially for other people.
Comment 39•11 years ago
|
||
So if I read this correctly you dumped the great feature of showing thumbnail previews of open tabs? I heavily rely upon this and thought it was just broke in release 2x. So no chances of it coming back? I agree, browser.allTabs.previews=true was one of the best features! please return it back!
Comment 40•11 years ago
|
||
I appreciate it's annoying to have to use a plugin for this but the plugin is very good - development and support have been excellent, it includes options for hotkeys, cursor focus location etc, so is now arguably better than the original feature: https://addons.mozilla.org/en-US/firefox/addon/all-tabs-restorer/
Comment 41•11 years ago
|
||
smug_master@hotmail.com, thanks for the add-on, it's almost as good.
Comment 42•10 years ago
|
||
Is there any code for this previous feature?
You need to log in
before you can comment on or make changes to this bug.
Description
•