Closed Bug 682507 Opened 10 years ago Closed 10 years ago

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


(Firefox :: Session Restore, enhancement)

Not set



Firefox 9


(Reporter: tabutils+bugzilla, Assigned: tabutils+bugzilla)


(Blocks 1 open bug)


(Keywords: dev-doc-complete)


(1 file, 2 obsolete files)

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.
Severity: normal → enhancement
Attached patch patch (obsolete) — Splinter Review
Attachment #560710 - Flags: review?(dietrich)
Comment on attachment 560710 [details] [diff] [review]

looks fine to me, f+. needs a test though. once you have a test for it, ask for final review.
Attachment #560710 - Flags: review?(dietrich) → feedback+
Is there any guideline to write a testcase? Thanks.
Attached patch patch v2 (obsolete) — Splinter Review
Attachment #560710 - Attachment is obsolete: true
Attachment #561507 - Flags: review?(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 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.
Attachment #561507 - Flags: review?(dietrich) → review+
Assignee: nobody → ithinc
Ever confirmed: true
Attachment #561507 - Attachment is obsolete: true
Keywords: checkin-needed
Flags: in-testsuite+
OS: Windows XP → All
Hardware: x86 → All
Comment on attachment 562304 [details] [diff] [review]
patch for checkin

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

>+  let ss = Cc[';1'].getService(Ci.nsISessionStore);

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

>+  executeSoon(finish);

executeSoon shouldn't be needed here.
In fact, finish should be removed altogether along with waitForExplicitFinish...
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 9
(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.
Blocks: 716279
You need to log in before you can comment on or make changes to this bug.