Last Comment Bug 564100 - No way to tell if a non-current tab has been read: [selected=false] is not possible anymore
: No way to tell if a non-current tab has been read: [selected=false] is not po...
Status: RESOLVED FIXED
[2.0-unaffected] [2.1-affected] [2.2-...
: regression
Product: SeaMonkey
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: Trunk
: All All
-- normal with 1 vote (vote)
: seamonkey2.4
Assigned To: Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention)
:
:
Mentors:
Depends on: 487242
Blocks: 133053 694740
  Show dependency treegraph
 
Reported: 2010-05-05 20:30 PDT by Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention)
Modified: 2011-10-15 00:11 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
unaffected
affected
fixed
fixed
fixed


Attachments
patch v0, raw port from bug 487242 att. 559597 (1.97 KB, patch)
2011-09-15 11:41 PDT, Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention)
neil: review+
bugspam.Callek: approval‑comm‑aurora+
bugspam.Callek: approval‑comm‑beta+
Details | Diff | Splinter Review

Description User image Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention) 2010-05-05 20:30:17 PDT
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a5pre) Gecko/20100505 SeaMonkey/2.1a1pre - Build ID: 20100505024302

In SeaMonkey 2.0.x and before, tabs were created without the "selected" attribute, which was added with the value true when making the tab current, and set to false when leaving the tab. This allowed an easy three-way test in userChrome.css:

*|tab:not([selected]) /* this tab has never been current yet in this session */
*|tab[selected=true]  /* this is the current tab */
*|tab[selected=false] /* this tab has been read but isn't current anymore */

In Sm2.1, when leaving a tab the selected attribute is removed instead, so the third selector above is never triggered (and the first one is triggered out of turn for non-current already-read tabs).
Comment 1 User image Philip Chee 2010-06-20 13:11:20 PDT
We need to port over something or other over from Firefox tabbrowser but I forget which.
Comment 2 User image Jens Hatlak (:InvisibleSmiley) 2011-02-06 03:32:22 PST
xref bug 487242 (has patch)
Comment 3 User image Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention) 2011-09-15 11:41:49 PDT
Created attachment 560423 [details] [diff] [review]
patch v0, raw port from bug 487242 att. 559597
Comment 4 User image Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention) 2011-09-15 11:59:47 PDT
Comment on attachment 560423 [details] [diff] [review]
patch v0, raw port from bug 487242 att. 559597

- not sure whom to ask for ui-review or if necessary (Neil, feel free to add)
- the aim of this patch is to allow to distinguish (e.g. in userChrome) which tabs haven't yet been seen
- in 2.0 and earlier, this was done by tab:not([selected])
- this patch aligns on the newly-implemented Firefox solution, tab[unread=true]
- for details, see comment #0 and bug 487242 attachment #559597 [details] [diff] [review]

oh, and BTW, I haven't tested all corner cases, and I'm not sure why SeaMonkey and Firefox tabbed browsers differ this much in their code.
Comment 5 User image neil@parkwaycc.co.uk 2011-09-15 16:29:53 PDT
Comment on attachment 560423 [details] [diff] [review]
patch v0, raw port from bug 487242 att. 559597

Seems reasonable.
Comment 6 User image Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention) 2011-09-15 21:16:03 PDT
The following remains to be tested:
- after session restore, does the first tab have the "unread" attribute if it is not selected? (bug 587242 comment #21)
- if it doesn't, a possible followup bug to bug 587242 may have to be ported.
- what about aurora, beta, etc.?
  - pro: low-risk change with no l10n impact;
         may affect theme writers and tech-savvy users with userChrome.css
         the Firefox patch is labeled "for m-c and aurora"
  - con: minor regression with low end-user visibility
         dates back to 2.1a1pre already
Comment 7 User image Justin Wood (:Callek) [away until Feb 27] 2011-09-15 21:44:02 PDT
Comment on attachment 560423 [details] [diff] [review]
patch v0, raw port from bug 487242 att. 559597

If this lands in next 6-12 hours, I can take it on beta, otherwise I suspect it will miss.
Comment 9 User image Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention) 2011-09-16 10:25:01 PDT
Mozilla/5.0 (X11; Linux x86_64; rv:9.0a1) Gecko/20110915 Firefox/9.0a1 SeaMonkey/2.6a1 ID:20110916032557

with the following in userChrome.css:

.tabbrowser-tabs tab:[unread=true]
  /* not yet read:                white           */
  { background-color:   white                   !important
  }
.tabbrowser-tabs tab:hover
  /* onmouseover:                 red             */
  { background-color:   #C00                    !important
  }
.tabbrowser-tabs tab[selected=true]
  /* current:                     aqua            */
  { background-color:   #0FF                    !important
  }
.tabbrowser-tabs tab[selected=true]:hover
  /* current on mouseover:        yellow          */
  { background-color:   yellow                  !important
  }

(the full stylesheet can be seen at http://users.skynet.be/antoine.mechelynck/other/userChrome-seamonkey.css )

the white beckgrounds are visible, except on the first tab, even after "restarting" (which is different from "closing down then starting up") with a tab other than the first being current.

We'll see how (and if) the Firefox guys fix this, see bug 487242 comment #23.
Comment 10 User image Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention) 2011-09-16 10:25:38 PDT
oops: no colon before [unread=true]
Comment 11 User image Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention) 2011-09-16 10:32:20 PDT
sorry, a non-transparent background on the first tab's favicon led me astray: it does have [unread=true] in the case I tested, as seen in DOM Inspector.
Comment 12 User image Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention) 2011-09-16 10:54:39 PDT
Hm, I think I see what happens, on restart with a current tab other than the first:
1. First, the tabs are created. The first tab doesn't have [unread], the others do.
2. Second, their favicons are read. The first tab still doesn't have [unread].
3. Finally, the contents are read. This is when the first tab, not being current, gets its [unread=true] attribute.
Comment 13 User image Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention) 2011-09-16 11:12:58 PDT
P.S. Of course, people who set browser.sessionstore.max_concurrent_tabs to 0 ("load tabs only on demand" instead of the default which is 3, "load them in background, no more than three at a time") will never get step 3 of comment #12 (the background load of the first tab contents) unless, maybe, they right-click the non-current first tab, then "Reload Tab".
Comment 14 User image Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention) 2011-09-19 21:14:38 PDT
see also bug 687754 (of whose wellfoundedness I'm not convinced).
Comment 15 User image Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention) 2011-09-19 21:17:29 PDT
...and see bug 687236, but I'm not sure I understand what it means.
Comment 16 User image Philip Chee 2011-10-09 21:41:23 PDT
tonymec: are you going to port Bug 687754 ([followup on bug 487242] unread attribute set at different times during page load) as well?
Comment 17 User image Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention) 2011-10-13 20:11:38 PDT
(In reply to Philip Chee from comment #16)
> tonymec: are you going to port Bug 687754 ([followup on bug 487242] unread
> attribute set at different times during page load) as well?

As time passes, I'm less and less convinced of that bug's wellfoundedness.

What happens ATM on Sm-trunk is as follows:
- Browser starts with one tab on about:blank. It is displayed, hence not set to unread.
- All tabs are opened (and not displayed, hence set to unread), then loaded (and still not displayed).
- If the "current tab" should be other than the first tab, that tab is made current (and, once it loads, loses "unread"). The first tab, still busy loading, isn't "unread" (its about:blank display has been shown).
- When the first tab loads, it becomes "unread" if it isn't current.

All this seems kosher to me, I don't see why I should meddle with it.
Comment 18 User image Len 2011-10-14 19:41:25 PDT
What attachment 565090 [details] [diff] [review] does is to move setting of "unread" to the moment when the document finished (re)loading from web (existing "busy" is removed) for all tabs. This makes tab2+ behave like tab1. Setting a new tab to unread before it has even loaded anything from the net was not intended and caused by the way additional tabs are opened.

It seems a minor annoyance but becomes important if "unread" is handled by sessionstore too (which is easily done by adding "unread" to the xulAttributes).
Than all tabs keep their read/unread status from one session to the next (loaded from cache only if restore_on_demand is set) and will become unread only if they are reloaded from the web.
Without that fix above, tab2+ would be set to unread as soon as they are opened. What should happen is, that sessionstore should load the content from cache keeping the stored read/unread and only set it new if the page was reloaded from net (see restore_on_demand).

There are many differences in tabbrowser and sessionstore between comm-central and mozilla-central.

Did a quick test myself in SM 2.5b3 with an adapted version of attachment 565090 [details] [diff] [review] plus added "unread" to xulAttributes in sessionstore. I didn't see what I expected. The patch in SM seemed to go only half the way it should. The differences in restoring a session and adding/loading tabs in the above mentioned files are bigger than I expected.
Needs someone with deeper knowledge of SM development and code to answer the questions, why m-c sessionstore can't be used and the 2 tabbrowser versions aren't closer together.
Comment 19 User image Tony Mechelynck [:tonymec]. (NEEDINFO me if you want my attention) 2011-10-14 23:27:13 PDT
(In reply to Len from comment #18)
[...]
> Needs someone with deeper knowledge of SM development and code to answer the
> questions, why m-c sessionstore can't be used and the 2 tabbrowser versions
> aren't closer together.

I suppose that might be Neil, the "SeaMonkey UI tsar", and he said over IRC that he would be ready to review a port of bug 687754 to SeaMonkey. I fear I'll be volunteered to do it against my feeling of what would be right.

This bug is FIXED, but it could be done in the followup, as on the Firefox side bug 687754 is a followup to bug 487242.

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