747338 - Set last-accessed timestamp when deselecting tabs rather than when selecting them

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v1

For bug 675539 we need to know when a tab was last selected (or visible).

Not sure if we need a setter for this? If session store is going to store this value (like in bug 739866) we might need to do that.

Comment 1

[Dão Gottwald [:dao]]

Comment on attachment 616926 [details] [diff] [review]
patch v1

Having both lastAccessed and lastSelected with different meanings is confusing. If the former isn't what we need, we should fix that.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Dão Gottwald [:dao] from comment #1)
> Having both lastAccessed and lastSelected with different meanings is
> confusing. If the former isn't what we need, we should fix that.

The former isn't what *I* need, but I agree that both are very (or too) similar. Not sure if bug 586067 could use lastSelected, too.

Comment 3

[Dão Gottwald [:dao]]

(In reply to Tim Taubert [:ttaubert] from comment #2)
> Not sure if bug 586067 could use lastSelected, too.

Yes. The point is to sort tabs in most-recently-used order.

Comment 4

[Nicolas Barbulesco]

“Set last-accessed timestamp when deselecting tabs rather than when selecting them”

I would rather say : both.

In case of crash, it's good to know that the current tab was accessed. The current tab had been selected, but not yet deselected.

Comment 5

[Dietrich Ayala (:dietrich)]

So the only reason this didn't land is because a minor naming concern?

Can we change to lastVisible and land?

Comment 6

[Dão Gottwald [:dao]]

Attachment: patch

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8371390 [details] [diff] [review]
patch

Review of attachment 8371390 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 8

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/fx-team/rev/52f33b9a2208

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/52f33b9a2208

Comment 10

[Dietrich Ayala (:dietrich)]

wooo, thanks y'all

Comment 11

[ithinc]

Comment on attachment 8371390 [details] [diff] [review]
patch

Review of attachment 8371390 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/tabbrowser.xml
@@ +4720,5 @@
>        </property>
>  
> +      <property name="lastAccessed">
> +        <getter>
> +          return this.selected ? Date.now() : this._lastAccessed;

Seems problematic in preview mode. We may set the time stamp on selected tab to Infinity.