Last Comment Bug 769097 - Remember desktop mode preference after OOM
: Remember desktop mode preference after OOM
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: -- normal (vote)
: Firefox 16
Assigned To: Brian Nicholson (:bnicholson)
:
:
Mentors:
Depends on:
Blocks: 766406
  Show dependency treegraph
 
Reported: 2012-06-27 16:54 PDT by Brian Nicholson (:bnicholson)
Modified: 2012-07-11 23:21 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
patch (6.10 KB, patch)
2012-06-27 16:55 PDT, Brian Nicholson (:bnicholson)
mark.finkle: review+
Details | Diff | Splinter Review

Description Brian Nicholson (:bnicholson) 2012-06-27 16:54:58 PDT
We should maintain the desktop mode after restoring from OOM.
Comment 1 Brian Nicholson (:bnicholson) 2012-06-27 16:55:26 PDT
Created attachment 637307 [details] [diff] [review]
patch
Comment 2 Mark Finkle (:mfinkle) (use needinfo?) 2012-06-30 06:46:36 PDT
Comment on attachment 637307 [details] [diff] [review]
patch

>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js

>     this.id = ++gTabIDFactory;
>+    this.desktopMode = aParams.desktopMode == true;

Use parens to make this a bit more readable:

      this.desktopMode = (aParams.desktopMode == true);

but I think we need more since desktopMode is optional and we want to be more explicit:

      this.desktopMode = ("desktopMode" in aParams) ? aParams.desktopMode : false;

r+ with the nits
Comment 3 Mark Finkle (:mfinkle) (use needinfo?) 2012-06-30 06:49:16 PDT
Comment on attachment 637307 [details] [diff] [review]
patch

>diff --git a/mobile/android/components/SessionStore.js b/mobile/android/components/SessionStore.js

>-          let params = { selected: isSelected, delayLoad: true, title: entry.title };
>+          let params = {
>+            selected: isSelected,
>+            delayLoad: true,
>+            title: entry.title,
>+            desktopMode: tabData.desktopMode == true
>+          };

Actually, what if the sessionstore.js file has no "desktopMode" data? Will this throw an exception?

I guess it won't and desktopMode would be null which evaluates to false. So we are OK.
Comment 4 Brian Nicholson (:bnicholson) 2012-07-02 12:42:51 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/4a5f794edd2d
Comment 5 Ryan VanderMeulen [:RyanVM] 2012-07-02 19:01:28 PDT
https://hg.mozilla.org/mozilla-central/rev/4a5f794edd2d
Comment 6 Brian Nicholson (:bnicholson) 2012-07-11 23:21:42 PDT
Uplifted as part of bug 766406.

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