682507 - Expose tab.linkedBrowser.__SS_restoreState as a tab attribute to help style unloaded tabs

Status: RESOLVED FIXED

(Firefox :: Session Restore, enhancement)

Description

[ithinc]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:6.0) Gecko/20100101 Firefox/6.0
Build ID: 20110811165603

Steps to reproduce:

1. Set browser.sessionstore.max_concurrent_tabs to 0 in about:config
2. Restart Firefox




Expected results:

I want to style the unloaded tabs with a lower opacity. Adding an attribute like "restoring" to reflect __SS_restoreState would help.

Comment 1

[ithinc]

Attachment: patch

Comment 2

[Dietrich Ayala (:dietrich)]

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

looks fine to me, f+. needs a test though. once you have a test for it, ask for final review.

Comment 3

[ithinc]

Is there any guideline to write a testcase? Thanks.

Comment 4

[ithinc]

Attachment: patch v2

Comment 5

[Dietrich Ayala (:dietrich)]

Sorry, I wasn't cc'd on the bug, so didn't see the follow-up comment! For future reference, check out the tests directories where the sessionstore code is - lots of examples there. Though, I presume that's what you did :)

Comment 6

[Dietrich Ayala (:dietrich)]

Comment on attachment 561507 [details] [diff] [review]
patch v2

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

::: browser/components/sessionstore/test/browser/browser_682507.js
@@ +4,5 @@
> +function test() {
> +  waitForExplicitFinish();
> +
> +  Services.prefs.setBoolPref("browser.sessionstore.restore_on_demand", true);
> +  gBrowser.addTab("about:mozilla");

looks ok, r=me.

my one concern was that in the case where you actually depend on the web content loading in the tab, you'd want to listen for some event before continuing on with the test. however, in this case, it doesn't really matter.

Comment 7

[ithinc]

Attachment: patch for checkin

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 562304 [details] [diff] [review]
patch for checkin

>+++ b/browser/components/sessionstore/test/browser/browser_682507.js

>+  let ss = Cc['@mozilla.org/browser/sessionstore;1'].getService(Ci.nsISessionStore);

ss should already be available, head.js sets it.

>+  executeSoon(finish);

executeSoon shouldn't be needed here.

Comment 9

[Dão Gottwald [:dao]]

In fact, finish should be removed altogether along with waitForExplicitFinish...

Comment 10

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e1be3e4479c3

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/mozilla-central/rev/e1be3e4479c3

Comment 12

[ithinc]

(In reply to Dão Gottwald [:dao] from comment #9)
> In fact, finish should be removed altogether along with
> waitForExplicitFinish...

Thanks for the corrections. This is my first testcase.

Comment 13

[Eric Shepherd [:sheppy]]

Documented:

https://developer.mozilla.org/en/XUL/Attribute/pending

Updated:

https://developer.mozilla.org/en/Firefox/Updating_add-ons_for_Firefox_9#Theme_changes
https://developer.mozilla.org/en/XUL/tab

And listed on Firefox 9 for developers.