Closed
Bug 487242
Opened 16 years ago
Closed 13 years ago
Regression: in userChrome.css no longer possible to distinguish between unvisited tabs, visited tabs, and the selected tab
Categories
(Firefox :: Tabbed Browser, defect)
Tracking
()
RESOLVED
FIXED
Firefox 9
People
(Reporter: bugdickvl, Assigned: lenniger)
References
(Blocks 2 open bugs)
Details
(Keywords: dev-doc-complete)
Attachments
(1 file, 7 obsolete files)
1.56 KB,
patch
|
dao
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090406 Minefield/3.6a1pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090406 Minefield/3.6a1pre
It used to be possible to place code in userChrome.css to differentiate between the currently selected tab and tabs that have been visited and tabs that hadn't been visited (opened in the background).
The current code makes this no longer possible.
Reproducible: Always
From: Bug 480813#c1 (dickvl)
I would prefer the old code in toolkit.jar/content/global/bindings/tabbox.xml
for the "selected" attribute to make it possible to see which tabs have been
visited (selected="false").
Convenient to see which tabs have been visited (e.g. forum threads) as posted
in Bug 471921.
Currently in Minefield (638):
if (val)
this.setAttribute("selected", "true");
else
this.removeAttribute("selected");
Code used in Firefox 3.0.x (637):
this.setAttribute("selected", val);
480813#c2(Dão Gottwald): (In reply to comment #1)
I don't think it makes sense to add that hack back, especially as it didn't
work reliably. We could add an attribute specifically for that use case,
though. For that, please file a new bug.
------
I agree that it didn't work reliably and agree that adding a separate visited attribute would work better:
if (val) {
this.setAttribute("selected", "true");
this.setAttribute("visited", "true");
{
else
this.removeAttribute("selected");
This version doesn't have the disadvantage that the other has.
I've tested it and it makes it possible to use undo close tab or dragging a tab without having other tabs getting a selected attribute.
It makes it easy to add code to userChrome.css to give visited tabs another color.
This is some of the code that I currently use:
.tabbrowser-tab {
-moz-appearance:none!important;
border:1px solid ThreeDShadow!important;
margin-left:0px!important;
}
/* Change color of not visited inactive tabs */
.tabbrowser-tab:not([visited]) {
color:#c09!important; font-style:normal!important;
font-size:10pt!important;
font-weight:bold!important;
}
/* Change color of visited inactive tabs */
.tabbrowser-tab[visited="true"] {
color:#03f!important;
font-style:normal!important;
font-size:10pt!important; font-weight:bold!important;
}
/* Change color of selected tab */
.tabbrowser-tab[selected="true"] {
color:#000!important;
font-style:normal!important;
font-size:10pt!important;
}
.tabbrowser-tab:not([busy]):not([visited]) {background-color:#9d6!important;}
.tabbrowser-tab:not([busy])[visited="true"] {background-color:#fcc!important;}
.tabbrowser-tab:not([busy])[selected="true"] {background-color:#ff9!important;}
.tabbrowser-tab[busy] {color:#ff0!important; background-color:#06c!important;}
.tabbrowser-tab[busy][selected="true"] {color:#00f!important; background-color:#0cf!important;}
Comment 1•15 years ago
|
||
I hope when this is fixed in Minefield that we will be shown
an example of visited, unvisited, and active; and even better
if the example can be coded so it works on all Firefox versions
if that is possible. Funny I'd never
noticed that restoring a tab made all unvisited tabs look visited.
I was certainly aware that restoring a session made all but
the active tab look unvisited which makes sense.
Tab Color Underscoring active/read/unread | userstyles.org
http://userstyles.org/styles/9023
Updated•15 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [good first bug]
Because there is not much progression in this bug I decided to give it a try and I've created an extension to add a 'visited' attribute to tabs that have been visited.
I'm not an extension developer, so if someone can review the code to see if this is a correct way and if not propose better code then this bug can be set to resolved and closed.
Tab Mix Plus also adds a visited attribute, so if you use TMP then this extension is not needed and the same code in userChrome.css should work with the TMP and TabVisted extension.
I've set the minVersion to 3.0 and the maxVersion to 3.* and this extension will also fix the buggy behavior in Firefox 3.0.x and 3.5.x versions that causes all tabs to get a selected attribute if you drag a tab or use Undo Closed Tabs to restore a closed tab.
Use :not([visited]) instead of :not([selected]) and [visited="true"] instead of :not([selected="true"]) and [selected="true"] in this order.
I've created two versions of the code and both work for me in all Firefox 3 versions.
This is the first version:
var TV_eventListener = {
init: function () {
window.addEventListener("load", this, false);
},
handleEvent: function(e) {
switch (e.type) {
case "TabSelect":
var tab = e.target;
if (!tab.hasAttribute("visited")) tab.setAttribute("visited", true);
break;
case "load":
setTimeout( function() {
var tab = gBrowser.selectedTab;
if (!tab.hasAttribute("visited")) tab.setAttribute("visited", true);
}, 0);
gBrowser.mTabContainer.addEventListener('TabSelect',this, false);
window.removeEventListener("load", this, false);
window.addEventListener("unload", this, false);
break;
case "unload":
gBrowser.mTabContainer.removeEventListener('TabSelect',this, false);
window.removeEventListener("unload", this, false);
break;
}
}
}
TV_eventListener.init();
This is a second version with slimmer code.
var tabsvisited = {
init: function() {
gBrowser.mTabContainer.addEventListener('TabSelect', function(e) {
var tab = e.target;
if (!tab.hasAttribute("visited")) tab.setAttribute("visited", true);
}, false);
setTimeout( function() {
var tab = gBrowser.selectedTab;
if (!tab.hasAttribute("visited")) tab.setAttribute("visited", true);
}, 0);
window.removeEventListener("load", tabsvisited.init, false);
}
}
window.addEventListener("load", tabsvisited.init, false);
I would appreciate to get some comments if these are correct ways to do it and if not give some suggestions for improvement or links to documents or examples. I see this as a way to learn something new as I've never done code like this before.
What is the best way to initialize such an object?
(In reply to comment #3)
> Created an attachment (id=419176) [details]
> TabVisted extension
Works very well here. I've been missing that feature for ages so thank you very very much! I tried it myself once but couldn't get anything to work.
I'm using this rule:
.tabbrowser-tab:not([visited]) .tab-text:not([value="Problem loading page"]):not([value^="Loading"])
{ color: -moz-hyperlinktext !important; }
Thanks again!
I've received a comment from Dao for improvement and simplification of the code, so here is an update of the previous version that I've tested for the past week without problems.
The setTimeout call is needed, without it the first tab always gets a visited state when restoring multiple tabs.
Of course I'm still in for a solution without this extension, maybe in Firefox 4.0 or 3.7.
var TV_eventListener = {
init: function () {
window.addEventListener("load", this, false);
},
handleEvent: function(e) {
switch (e.type) {
case "TabSelect":
e.target.setAttribute("visited", true);
break;
case "load":
setTimeout( function() {gBrowser.selectedTab.setAttribute("visited", true);}, 0);
gBrowser.tabContainer.addEventListener('TabSelect',this, false);
}
}
}
TV_eventListener.init();
Comment 7•15 years ago
|
||
Thanks dickvl changes work in my style 24728, hope the code you and Dao worked on gets into Firefox 3.6.x
I don't know if I can modify http://userstyles.org/styles/9023 to work with Firefox 3.6 as well as prior versions of Firefox so I created http://userstyles.org/styles/24728
Updated•14 years ago
|
Assignee: nobody → dao
Whiteboard: [good first bug]
Comment 8•14 years ago
|
||
userChrome.css code:
.tabbrowser-tab[unread] {
font-style: italic;
}
Attachment #419176 -
Attachment is obsolete: true
Attachment #420804 -
Attachment is obsolete: true
Attachment #456026 -
Flags: review?(gavin.sharp)
I tested the patch in FF 3.6.10 and it seems to work as expected for new tabs.
Do we need the "unread" for a new empty page (about:blank)? Before any content is loaded? If not, the suggested change below should make the change in addTab obsolete (maybe the change in browser.js too).
What I am missing is an "unread" setting for previous selected tabs after they are unselected and reloaded. After a reload the content is 'new' and so it is unread like a new tab.
The idea is to catch the end of the tab being busy and than set it to unread if its not selected. If the tab gets selected, remove unread again. Straight and simple.
An additional 2-liner in tabbrowser.xml / mTabProgressListener / onStateChange can do that.
Right after
this.mTab.removeAttribute("busy")
insert
if (!this.mTab.hasAttribute("selected"))
this.mTab.setAttribute("unread", "true")
This makes it simple and catches all cases where an unselected tab is (re)loaded.
Assignee | ||
Comment 10•14 years ago
|
||
Modify the 2-liner above to
if (this.mTabBrowser.mCurrentTab != this.mTab)
this.mTab.setAttribute("unread", "true");
Better and tested to work in FF 3.5.15 too.
Comment 11•14 years ago
|
||
As workaround, using the extension userChromeJS, you could use this script:
if (location == "chrome://browser/content/browser.xul") {
visitedtab = function (event) {
let index = event.target._tPos;
let tab=document.querySelectorAll('.tabbrowser-tab')[index];
tab.setAttribute('visited','true');
}
document.querySelectorAll('.tabbrowser-tab')[0].setAttribute('visited','true');
let container = gBrowser.tabContainer;
container.addEventListener("TabSelect", visitedtab, true);
}
and putting this code in your userChrome.css:
.tabbrowser-tab:not([visited]) .tab-text{
style
}
I have tested it in Firefox 4 RC1.
Comment 12•13 years ago
|
||
Comment on attachment 456026 [details] [diff] [review]
patch
The browser.js change seems fairly hacky - although it appears that the selectedTab restoration still does happen synchronously under init(), I don't really like relying on that (given that sessionstore has moved towards doing restoration in chunks). How about just having sesionstore deal with this particular case itself?
I wonder whether we should avoid resetting the attribute in _previewMode.
Attachment #456026 -
Flags: review?(gavin.sharp) → review-
Assignee | ||
Comment 13•13 years ago
|
||
I don't see why an emtpy new tab needs to be marked as unread.
A tab reloaded after is was selected should be marked unread because it's content could have been changed by reloading.
Using a different approach avoids the browser.js change too.
It sets the read attribute if a tab finishes loading and removes the attribute if the tab is selected. Very unintrusive.
Using it myself since last autumn.
Attachment #558195 -
Attachment description: unread - different approach → unread - different approach, tabbrowser.xml only
Comment 14•13 years ago
|
||
Comment on attachment 558195 [details]
unread - different approach, tabbrowser.xml only
>+ if (this.mTabBrowser.mCurrentTab != this.mTab)
if (!this.mTab.selected)
Updated•13 years ago
|
Assignee: dao → lenniger
Updated•13 years ago
|
Attachment #456026 -
Attachment is obsolete: true
Assignee | ||
Comment 15•13 years ago
|
||
I agree, it's cleaner.
Attachment #559030 -
Flags: review?(gavin.sharp)
Attachment #558195 -
Attachment is obsolete: true
Attachment #558195 -
Attachment is patch: false
Updated•13 years ago
|
Attachment #559030 -
Flags: review?(gavin.sharp) → review?(dao)
Comment 16•13 years ago
|
||
Comment on attachment 559030 [details] [diff] [review]
unread - tabbrowser.xml only v2
This patch doesn't apply. It's not based on mozilla-central, is it?
Attachment #559030 -
Flags: review?(dao)
Assignee | ||
Comment 17•13 years ago
|
||
No, it is based on mozilla-1.9.2 where the bug was originally filed for.
For the other branches they look slightly different.
Assignee | ||
Comment 18•13 years ago
|
||
version for mozilla-central and mozilla-aurora
Assignee | ||
Comment 19•13 years ago
|
||
version for mozilla-beta and mozilla-release
Comment 20•13 years ago
|
||
(In reply to David McRitchie from comment #1)
> I hope when this is fixed in Minefield that we will be shown
> an example of visited, unvisited, and active; and even better
> if the example can be coded so it works on all Firefox versions
> if that is possible. Funny I'd never
> noticed that restoring a tab made all unvisited tabs look visited.
> I was certainly aware that restoring a session made all but
> the active tab look unvisited which makes sense.
The following is an extract from my Firefox userChrome.css dated 10 June 2009 (based on the moz-1.9.1 and earlier criteria):
/*
* define various backgrounds for tabs
* (the first one doesn't work anymore,
* see bug 487242)
*/
.tabbrowser-tabs *|tab:not(selected) /* unread */
{ background-color: white !important
}
.tabbrowser-tabs *|tab:hover /* mouseover */
{ background-color: #C00 !important
}
.tabbrowser-tabs *|tab[selected=true] /* selected */
{ background-color: #699 !important
}
.tabbrowser-tabs *|tab[selected=true]:hover /* both */
{ background-color: yellow !important
}
IIUC, to make it work with the current version of the patch (which adds an "unread" attribute), the only needed change would be the replacement of :not([selected]) by [unread=true] in the first selector.
However, I don't think that placing both versions of the selector (comma-separated of course) in the same CSS style sheet would work. This, however, can be considered a moot point since who uses moz-1.9.1 builds of Firefox anymore?
Updated•13 years ago
|
Attachment #559030 -
Attachment is obsolete: true
Updated•13 years ago
|
Attachment #559612 -
Attachment is obsolete: true
Updated•13 years ago
|
Attachment #559597 -
Flags: review?(dao)
Comment 21•13 years ago
|
||
Comment on attachment 559597 [details] [diff] [review]
unread - v2c (for mozilla-central and aurora)
The only glitch I noticed here is that after session restore, the first tab doesn't have the 'unread' attribute even if it's not selected. Could you please file a followup bug on this?
Attachment #559597 -
Flags: review?(dao) → review+
Comment 22•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 9
Updated•13 years ago
|
Keywords: dev-doc-needed
Comment 23•13 years ago
|
||
(In reply to Dão Gottwald [:dao] from comment #21)
> Comment on attachment 559597 [details] [diff] [review]
> unread - v2c (for mozilla-central and aurora)
>
> The only glitch I noticed here is that after session restore, the first tab
> doesn't have the 'unread' attribute even if it's not selected. Could you
> please file a followup bug on this?
...and mention the followup bug number not only here, but also on SeaMonkey bug 564100 (SeaMonkey version of this Firefox bug, which suffers from the same "glitch"). Thanks.
Comment 24•13 years ago
|
||
Dão: On SeaMonkey, with an identical patch as this one, the first tab, when not current at startup, gets its [unread=true] attribute, but belatedly (the other tabs when they are created, the first tab when its contents are actually read): see bug 564100 comment #11 and #12. I have browser.sessionstore.max_concurrent_tabs — default — integer — 3 (for people who set it at zero to load tabs "only when needed", the background load of the first tab's contents will never happen IIUC).
Comment 25•13 years ago
|
||
So I guess we may remove the "titlechanged" attribute and use "unread" instead? And Gavin's concern is valid.
(In reply to Gavin Sharp from comment #12)
> I wonder whether we should avoid resetting the attribute in _previewMode.
Comment 26•13 years ago
|
||
(In reply to Tony Mechelynck [:tonymec] from comment #24)
> the first tab, when
> not current at startup, gets its [unread=true] attribute, but belatedly
Confirming this. Marking "unread" on DOMTitleChanged seems to behave better.
Assignee | ||
Comment 27•13 years ago
|
||
Over the weekend I was investigating with different FF versions (6 - 9) the 3 points noted.
1) first tab not getting 'unread' after session restore (Dão Gottwald comment #21):
Could not reproduce it a single time. Maybe the tab needed very long to load the page and wasn't finished?
2) first tab gets 'unread' later than the other tabs (Tony Mechelynck comment #24, myself):
The first tab gets the attribute when it's finished loading (like it should), other tabs are getting it earlier (too early). Reason is the first tab is loaded different than the others. Tabs already have the busy attribute while they are loading, 'unread' is indicating a tab not selected after it was loaded.
3) avoid resetting 'read' in _previewMode (Gavin Sharp comment #12, ithinc comment #25):
Can't test it on Win7 at the moment but some more code reading was helpful I think.
Uploading fixes for points 2 and 3 in a little bit.
Comment 28•13 years ago
|
||
(In reply to Len from comment #27)
[...]
> 2) first tab gets 'unread' later than the other tabs (Tony Mechelynck
> comment #24, myself):
> The first tab gets the attribute when it's finished loading (like it
> should), other tabs are getting it earlier (too early). Reason is the first
> tab is loaded different than the others. Tabs already have the busy
> attribute while they are loading, 'unread' is indicating a tab not selected
> after it was loaded.
[...]
I wouldn't say it's too early. They have never been read, after all, and (except the current tab) are not currently pending to be seen as soon as they'll be loaded. However, who am I? I'll defer to owners and peers of the relevant module (and since Fx Tabbed Browser isn't owned IIUC, then Fx General UI or something): Dão? Gavin? aromano?
Assignee | ||
Comment 29•13 years ago
|
||
Fixes for early setting of attribute in tab 2+ and resetting in_previewMode.
Can someone please test and confirm that _previewMode in Win7 is no longer resetting the unread attribute?
Comment 30•13 years ago
|
||
Comment on attachment 561071 [details] [diff] [review]
unread - v2c add (mozilla-central)
Could you please file a separate followup bug on any outstanding issue? Thanks!
Attachment #561071 -
Attachment is obsolete: true
Assignee | ||
Comment 31•13 years ago
|
||
(In reply to Tony Mechelynck [:tonymec] from comment #28)
You can catch the start of loading by using the busy attribute. So if you want a tab colored from the beginning of loading, you can additionally set it for the combination busy + not selected.
If you are talking about session restore, that is a different case (save and restore attributes) and needs to be handled there. If the unread attribute is saved and restored, you get the proper color mark even with 'load tabs on demand'.
(In reply to Dão Gottwald [:dao] from comment #30)
> Could you please file a separate followup bug on any outstanding issue?
Followup: Bug 687754
Comment 32•13 years ago
|
||
Am I correct that this adds an "unread" attribute that is present and true on tabs that have yet to be the current tab since the start of the session?
Assignee | ||
Comment 33•13 years ago
|
||
It works during a session too:
New and reloaded tabs will get that attribute too if they are not current.
It adds the "unread" to a (re)loaded tab that isn't current.
Selecting the tab removes "unread" again.
So the attribute indicates if a tab hasn't been selected/current since it was (re)loaded, allowing to style them different (visual indication of unread tabs).
Comment 34•13 years ago
|
||
Glad I asked then!
Documented:
https://developer.mozilla.org/en/XUL/Attribute/unread
Listed on https://developer.mozilla.org/en/XUL/tab and Firefox 9 for developers.
Also mentioned here: https://developer.mozilla.org/en/Firefox/Updating_add-ons_for_Firefox_9#Theme_changes
Keywords: dev-doc-needed → dev-doc-complete
Comment 35•13 years ago
|
||
Shouldn't this attribute be set for all tabs when restoring a session? Right now it seems that the attribute isn't kept between restarts, and after restart, Firefox considers all tabs as being read, which can't be right. I guess?
But then, I'm using the Session Manager extension; is it something that should be set by the extension rather than by Firefox?
Comment 36•13 years ago
|
||
Oh wait, reading https://developer.mozilla.org/en/XUL/tab (thanks Eric!), I can see that this case is handled by the [pending] attribute. Although I don't know if a [pending] tab should or shouldn't, by definition, also be [unread].
Assignee | ||
Comment 37•13 years ago
|
||
If you use "Don't load tabs until selected", it's correct that all pending tabs (not yet loaded) have no unread attribute. That attribute is set after content is loaded into a tab that is not selected. Having a tab marked unread if no content is loaded into it? I don't think it makes sense simply because the is nothing to read.
The situation changes is "unread" is carried over between sessions. Than a pending tab will be "unread" too if it was marked in the restored session.
There is Bug 687237 filed for saving and restoring the unread attribute between sessions. Until Bug 687237 is fixed, there is a workaround: https://developer.mozilla.org/en/nsISessionStore#persistTabAttribute() can be used to add "unread" to the saved/restored XUL attributes.
Unselect "Don't load tabs until selected" and you will see all (except the current) tabs marked unread after they finished loading after a session restart.
You need to log in
before you can comment on or make changes to this bug.
Description
•