Closed Bug 1588874 Opened 6 months ago Closed 5 months ago

Remove preprocessing from nsSessionStore.js in SeaMonkey

Categories

(SeaMonkey :: Session Restore, enhancement)

All
macOS
enhancement
Not set

Tracking

(seamonkey2.49esr wontfix, seamonkey2.53 affected, seamonkey2.57esr fixed, seamonkey2.63 wontfix)

RESOLVED FIXED
seamonkey2.69
Tracking Status
seamonkey2.49esr --- wontfix
seamonkey2.53 --- affected
seamonkey2.57esr --- fixed
seamonkey2.63 --- wontfix

People

(Reporter: frg, Assigned: frg)

References

Details

Attachments

(1 file)

Just in the way for easy testing.

See Bug 1228655 "Replace #ifdefs in browser code by AppConstants".

Tested in 2.53. z-order under mac seems to work now. RecentWindow.jsm uses more or less the same enumerator code.

Attachment #9102849 - Flags: review?(iann_bugzilla)
Attachment #9102849 - Flags: approval-comm-release?
Attachment #9102849 - Flags: approval-comm-esr60?
Comment on attachment 9102849 [details] [diff] [review]
1588874-killpreprocessing.patch

>+++ b/suite/components/sessionstore/nsSessionStore.js

>-#ifndef XP_MACOSX
>-        if (!this._doResumeSession()) {
>-#endif
>+        if (AppConstants.platform == "macosx" || !this._doResumeSession()) {
Shouldn't this be != rather than == ?

r/a=me with that addressed.
Attachment #9102849 - Flags: review?(iann_bugzilla)
Attachment #9102849 - Flags: review+
Attachment #9102849 - Flags: approval-comm-release?
Attachment #9102849 - Flags: approval-comm-release+
Attachment #9102849 - Flags: approval-comm-esr60?
Attachment #9102849 - Flags: approval-comm-esr60+

(In reply to Ian Neal from comment #2)

Comment on attachment 9102849 [details] [diff] [review]

-#ifndef XP_MACOSX
-        if (!this._doResumeSession()) {
-#endif
+        if (AppConstants.platform == "macosx" || !this._doResumeSession()) {

Shouldn't this be != rather than == ?
No, the old code always ran this block on the Mac rather than the else block which was #ifdef'd out. (Might it be clearer to switch the blocks around and use if (AppConstants.platform != "macosx" && this._doResumeSession())?

(Might it be clearer to switch the blocks

Our new code is the same as Fx does it now:
https://dxr.mozilla.org/comm-central/source/browser/components/sessionstore/SessionStore.jsm#1221

Our sessionstore code needs a makeover. Switched would probably be better but there seems to be no longer an else in Mozilla browser code. So better let it stay for now to ease backporting.

Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/128a2ee8274f
Do not use preprocessing in nsSessionStore.js. r=IanN

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Blocks: 1160782
You need to log in before you can comment on or make changes to this bug.