340895 - Move SessionStore preferences to firefox.js

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect)

Description

[Simon Bünzli]

... and get rid of _getPref and DEFAULT_* altogether.

This is what most other components do and would at least make it obvious to testers what defaults were chosen (i.e. that it's currently disabled and that POSTDATA won't be saved).

Comment 1

[Simon Bünzli]

*** Bug 343151 has been marked as a duplicate of this bug. ***

Comment 2

[Simon Bünzli]

Attachment: fix

Comment 3

[Simon Bünzli]

Attachment: browser.js bits

Comment 4

[Mike Connor [:mconnor]]

Will likely still take this, but not going to block on code cleanup.

Comment 5

[logan]

Attachment: .startup.page firefox.js bits

just updates browser.startup.page notes in firefox.js after bug 340898

Comment 6

[Simon Bünzli]

Attachment: all-in-one patch

Asking review from Dietrich since he's probably got more time for this.

Comment 7

[Asaf Romano (gone)]

Comment on attachment 229881 [details] [diff] [review]
all-in-one patch

Simon, could you please update this patch to current trunk? thanks.

Comment 8

[Simon Bünzli]

Attachment: fix (unbitrotted)

Comment 9

[Asaf Romano (gone)]

Comment on attachment 233498 [details] [diff] [review]
fix (unbitrotted)

r=mano (trunk only for now).

Comment 10

[Peter Kasting]

Checked in on trunk.  Request approval as necessary if you want this landed on branch.

/mozilla/browser/components/sessionstore/src/nsSessionStore.js 1.37
/mozilla/browser/components/sessionstore/src/nsSessionStartup.js 1.11
/mozilla/browser/app/profile/firefox.js 1.147
/mozilla/browser/base/content/browser.js 1.685

Comment 11

[Peter Kasting]

Backed out due to tinderbox orange.

Comment 12

[Simon Bünzli]

Attachment: fix (unbitrotted 2)

(In reply to comment #11)
> Backed out due to tinderbox orange.

Any idea what might have caused this? In case it was just bitrot - please land this updated patch.

Comment 13

[Adam Guthrie]

I don't think it was bitrot; I think it had something to do with moving the prefs. to firefox.js -- the startup tests were failing in the same way they did when session store was first enabled.

pkasting, rhelmer and I discussed this on IRC. I don't know if they ever figured out what was /actually/ going on, though.

Comment 14

[Simon Bünzli]

(In reply to comment #13)
> I don't think it was bitrot; I think it had something to do with moving the
> prefs. to firefox.js -- the startup tests were failing in the same way they did
> when session store was first enabled.

IIRC the startup tests were failing because SessionStore tried to restore the previous session since Firefox is simply killed after each test. To that end we had introduced the browser.sessionstore.resume_from_crash pref which is set to false by the test setup script.

Adding that pref to firefox.js should however not have any effect since the setup script already changes the value of other prefs having a default value. I'd thus appreciate it if anybody with better knowledge about the testing infrastructure could shed some insight on the matter.

Comment 15

[:Gavin Sharp [email: [e-mail]]]

Simon, could you post an updated patch that applies to current trunk?

Comment 16

[Simon Bünzli]

Attachment: fix (unbitrotted 3)

Comment 17

[Dietrich Ayala (:dietrich)]

*** Bug 363650 has been marked as a duplicate of this bug. ***

Comment 18

[Dietrich Ayala (:dietrich)]

Attachment: fix (unbitrotted 4)

unbitrotted, and fixed syntax error.

Comment 19

[Daniel Veditz [:dveditz]]

Bug 363650 was marked a duplicate of this (sorry about that, I did search), but for the FF2 updates I only want the firefox.js changes, not the code cleanup (defaults in the code aren't going to hurt anything in FF2).

Comment 20

[Simon Bünzli]

Attachment: prefs only (branch patch)

Comment 21

[Asaf Romano (gone)]

Comment on attachment 248466 [details] [diff] [review]
fix (unbitrotted 4)

lets try landing this again.

Comment 22

[Jay Patel [:jay]]

Comment on attachment 249011 [details] [diff] [review]
prefs only (branch patch)

Approved for 1.8 branch, a=jay for drivers.

Comment 23

[:Gavin Sharp [email: [e-mail]]]

I was just about to check this in, but it got bitrotted by bug 364972 :(.

Comment 24

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/app/profile/firefox.js 	1.71.2.80

Comment 25

[Dietrich Ayala (:dietrich)]

i'm un-bitrotting the trunk patch now for check-in.

Comment 26

[Simon Bünzli]

Attachment: fix (unbitrotted 5)

Sorry 'bout the bitrot. Here we go again...

Comment 27

[Dietrich Ayala (:dietrich)]

Attachment: as checked-in on trunk

Comment 28

[Dietrich Ayala (:dietrich)]

Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--  firefox.js
new revision: 1.168; previous revision: 1.167
done
Checking in browser/base/content/browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.748; previous revision: 1.747
done
Checking in browser/components/sessionstore/src/nsSessionStartup.js;
/cvsroot/mozilla/browser/components/sessionstore/src/nsSessionStartup.js,v  <--  nsSessionStartup.js
new revision: 1.15; previous revision: 1.14
done
Checking in browser/components/sessionstore/src/nsSessionStore.js;
/cvsroot/mozilla/browser/components/sessionstore/src/nsSessionStore.js,v  <--  nsSessionStore.js
new revision: 1.57; previous revision: 1.56
done

Comment 29

[Michael Kohler [:mkohler]]

[VERIFIED] http://hg.mozilla.org/mozilla-central/file/7fcb443a08ce/browser/app/profile/firefox.js#l740