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)

x86
All
defect
Not set
normal

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)

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;}
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
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?
Attached file TabVisted extension (obsolete) —
(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();
Attached file TabVisited extension version 1.1 (obsolete) —
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
Blocks: 209179
Assignee: nobody → dao
Whiteboard: [good first bug]
Attached patch patch (obsolete) — Splinter Review
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.
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.
Blocks: 564100
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 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-
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 on attachment 558195 [details] unread - different approach, tabbrowser.xml only >+ if (this.mTabBrowser.mCurrentTab != this.mTab) if (!this.mTab.selected)
Assignee: dao → lenniger
Attachment #456026 - Attachment is obsolete: true
Attached patch unread - tabbrowser.xml only v2 (obsolete) — Splinter Review
I agree, it's cleaner.
Attachment #559030 - Flags: review?(gavin.sharp)
Attachment #558195 - Attachment is obsolete: true
Attachment #558195 - Attachment is patch: false
Attachment #559030 - Flags: review?(gavin.sharp) → review?(dao)
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)
No, it is based on mozilla-1.9.2 where the bug was originally filed for. For the other branches they look slightly different.
version for mozilla-central and mozilla-aurora
version for mozilla-beta and mozilla-release
(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?
Attachment #559030 - Attachment is obsolete: true
Attachment #559612 - Attachment is obsolete: true
Attachment #559597 - Flags: review?(dao)
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+
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 9
Keywords: dev-doc-needed
(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.
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).
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.
Blocks: 687236
Blocks: 687237
(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.
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.
(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?
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 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
(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
Blocks: 687754
Blocks: 133053
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?
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).
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?
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].
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.

Attachment

General

Creator:
Created:
Updated:
Size: