564100 - No way to tell if a non-current tab has been read: [selected=false] is not possible anymore

Status: RESOLVED FIXED

(SeaMonkey :: Tabbed Browser, defect)

Description

[Tony Mechelynck [:tonymec]]

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

[Philip Chee]

We need to port over something or other over from Firefox tabbrowser but I forget which.

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

xref bug 487242 (has patch)

Comment 3

[Tony Mechelynck [:tonymec]]

Attachment: patch v0, raw port from bug 487242 att. 559597

Comment 4

[Tony Mechelynck [:tonymec]]

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

[[e-mail]]

Comment on attachment 560423 [details] [diff] [review]
patch v0, raw port from bug 487242 att. 559597

Seems reasonable.

Comment 6

[Tony Mechelynck [:tonymec]]

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

[Justin Wood (:Callek)]

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 8

[Philip Chee]

Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/d31c11953f75
Pushed to comm-aurora:
http://hg.mozilla.org/releases/comm-aurora/rev/2c654b93aaa1
Pushed to comm-beta:
http://hg.mozilla.org/releases/comm-beta/rev/1c74bb2ef964

Comment 9

[Tony Mechelynck [:tonymec]]

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

[Tony Mechelynck [:tonymec]]

oops: no colon before [unread=true]

Comment 11

[Tony Mechelynck [:tonymec]]

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

[Tony Mechelynck [:tonymec]]

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

[Tony Mechelynck [:tonymec]]

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

[Tony Mechelynck [:tonymec]]

see also bug 687754 (of whose wellfoundedness I'm not convinced).

Comment 15

[Tony Mechelynck [:tonymec]]

...and see bug 687236, but I'm not sure I understand what it means.

Comment 16

[Philip Chee]

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

[Tony Mechelynck [:tonymec]]

(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

[Len]

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

[Tony Mechelynck [:tonymec]]

(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.