Last Comment Bug 682507 - Expose tab.linkedBrowser.__SS_restoreState as a tab attribute to help style unloaded tabs
: Expose tab.linkedBrowser.__SS_restoreState as a tab attribute to help style u...
: dev-doc-complete
Product: Firefox
Classification: Client Software
Component: Session Restore (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: Firefox 9
Assigned To: ithinc
: Mike de Boer [:mikedeboer]
Depends on:
Blocks: 716279
  Show dependency treegraph
Reported: 2011-08-26 21:49 PDT by ithinc
Modified: 2012-08-27 05:18 PDT (History)
4 users (show)
emorley: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (1.62 KB, patch)
2011-09-17 07:48 PDT, ithinc
dietrich: feedback+
Details | Diff | Splinter Review
patch v2 (4.21 KB, patch)
2011-09-21 10:16 PDT, ithinc
dietrich: review+
Details | Diff | Splinter Review
patch for checkin (4.54 KB, patch)
2011-09-25 08:16 PDT, ithinc
no flags Details | Diff | Splinter Review

Description ithinc 2011-08-26 21:49:25 PDT
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 2011-09-17 07:48:27 PDT
Created attachment 560710 [details] [diff] [review]
Comment 2 Dietrich Ayala (:dietrich) 2011-09-19 14:53:55 PDT
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.
Comment 3 ithinc 2011-09-20 02:48:12 PDT
Is there any guideline to write a testcase? Thanks.
Comment 4 ithinc 2011-09-21 10:16:31 PDT
Created attachment 561507 [details] [diff] [review]
patch v2
Comment 5 Dietrich Ayala (:dietrich) 2011-09-21 10:28:08 PDT
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) 2011-09-21 10:31:09 PDT
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 2011-09-25 08:16:12 PDT
Created attachment 562304 [details] [diff] [review]
patch for checkin
Comment 8 Dão Gottwald [:dao] 2011-09-27 01:36:14 PDT
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.
Comment 9 Dão Gottwald [:dao] 2011-09-27 01:38:20 PDT
In fact, finish should be removed altogether along with waitForExplicitFinish...
Comment 11 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-09-27 05:38:04 PDT
Comment 12 ithinc 2011-09-27 18:46:00 PDT
(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.

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