Closed
Bug 1260460
Opened 8 years ago
Closed 8 years ago
reset user's restore on demand preference during e10s migration
Categories
(Firefox :: Settings UI, defect)
Firefox
Settings UI
Tracking
()
RESOLVED
FIXED
Firefox 48
People
(Reporter: blassey, Assigned: Felipe)
References
Details
Attachments
(1 file)
58 bytes,
text/x-review-board-request
|
mconley
:
review+
ritu
:
approval-mozilla-aurora+
|
Details |
No description provided.
Reporter | ||
Updated•8 years ago
|
tracking-e10s:
--- → m9+
Comment 1•8 years ago
|
||
I have a followup in bug 1257554 to understand exactly why we're doing this for E10S, but assuming there's a need I don't think this should just be a pref reset. If this isn't working in E10S, we should just remove support for the pref entirely. I don't want to have a footgun pref (even if it's only in about:config).
Assignee | ||
Comment 2•8 years ago
|
||
Dolske, I brought this up on the e10s meeting yesterday with Blassey, Jeff & co. The consensus was that everyone agrees that this is a bad feature, but not strongly enough to remove it due to e10s. A pref reset is intended to avoid putting any users who might hit these edges cases in a bad situation without them being aware, while allowing people to keep using it if they desire to have it back. There's also some ongoing perf work that might alleviate the regressions and improve the situation.
Assignee | ||
Comment 3•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/43885/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/43885/
Attachment #8737300 -
Flags: review?(mconley)
Comment 4•8 years ago
|
||
Comment on attachment 8737300 [details] MozReview Request: Bug 1260460 - reset user's restore on demand preference during e10s migration. r=mconley https://reviewboard.mozilla.org/r/43885/#review40463 ::: browser/components/nsBrowserGlue.js:2170 (Diff revision 1) > "passwordCol", > "hidden"); > } > > + if (currentVersion < 37) { > + Services.prefs.clearUserPref("browser.sessionstore.restore_on_demand"); This is going to reset this pref for _everybody_, not just people who are migrating to using e10s. Are we sure we want to do that?
Attachment #8737300 -
Flags: review?(mconley)
Assignee | ||
Comment 5•8 years ago
|
||
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #4) > Comment on attachment 8737300 [details] > MozReview Request: Bug 1260460 - reset user's restore on demand preference > during e10s migration. r=mconley > > https://reviewboard.mozilla.org/r/43885/#review40463 > > ::: browser/components/nsBrowserGlue.js:2170 > (Diff revision 1) > > "passwordCol", > > "hidden"); > > } > > > > + if (currentVersion < 37) { > > + Services.prefs.clearUserPref("browser.sessionstore.restore_on_demand"); > > This is going to reset this pref for _everybody_, not just people who are > migrating to using e10s. Are we sure we want to do that? I thought so (at least, I did that intentionally), following the principle of not having any different behavior for e10s and non-e10s users.
Comment 6•8 years ago
|
||
K, gonna needinfo canuckistani to be absolutely sure.
Flags: needinfo?(jgriffiths)
Comment 7•8 years ago
|
||
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #6) > K, gonna needinfo canuckistani to be absolutely sure. I think we should reset for everyone, because we're removing the UI for everyone so we may be causing people to get locked into the pref being non-default if we remove the UI and do not reset the pref. Some people might find this confusing, so we should ensure that this change is well-documented in relnotes and on SUMO.
Flags: needinfo?(jgriffiths)
Comment 8•8 years ago
|
||
Comment on attachment 8737300 [details] MozReview Request: Bug 1260460 - reset user's restore on demand preference during e10s migration. r=mconley https://reviewboard.mozilla.org/r/43885/#review40493 Given canuckistani's comment, r=me. Thanks felipe!
Attachment #8737300 -
Flags: review+
Comment 11•8 years ago
|
||
Follow-up push: https://hg.mozilla.org/integration/fx-team/rev/6f4210b98c4b Replace "currentVersion" with "currentUIVersion" to fix XPCshell and browser-chrome bustage related to nsBrowserGlue.js. Initial push with test failures: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=43f2fc45eaf759727dfed6893a8c2efc8c35ddf9
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/43f2fc45eaf7 https://hg.mozilla.org/mozilla-central/rev/6f4210b98c4b
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Comment 13•8 years ago
|
||
it will be very sad to let it go instead of real fixing, I know it's startup & memory wise and I can restore session like before by just simply keep pressing Ctrl+Tab for a few sec, but it's only silly workaround for old feature [-( nearly the same thing was with awesome feature like loading tabs progressively... Many users including me use this feature & will have to consider vivaldi now
Comment 14•8 years ago
|
||
can tabs after session restore throttled and load 1–2 tabs simultaneously (user's pref of 1 or 100 whatever the user like) My office users use this pref a lot & a fall back would really help guys.
Assignee | ||
Comment 15•8 years ago
|
||
Hi, nothing is being removed here. The pref will just get reset, but you can go back to its previous value by just going to about:config and changing it (browser.sessionstore.restore_on_demand).
Comment 16•8 years ago
|
||
(In reply to :Felipe Gomes (needinfo me!) from comment #15) > Hi, nothing is being removed here. The pref will just get reset, but you can > go back to its previous value by just going to about:config and changing it > (browser.sessionstore.restore_on_demand). ah thanks for the info. https://bugzilla.mozilla.org/show_bug.cgi?id=1257554#c19 confused browser.sessionstore.restore_on_demand will be available right?
Assignee | ||
Comment 17•8 years ago
|
||
Yeah, maybe in the future it will be removed and replaced with something better, but for now that pref will remain available.
Comment 18•8 years ago
|
||
(In reply to :Felipe Gomes (needinfo me!) from comment #17) > Yeah, maybe in the future it will be removed and replaced with something > better, but for now that pref will remain available. thank you :) Most of our users want/like it so have to deploy it. Glad that there is a pref. in future version > can tabs after session restore throttled and load 1–2 tabs simultaneously > (user's pref of 1 or 100 whatever the user like) > > My office users use this pref a lot & a fall back would really help guys. :)
Comment 19•8 years ago
|
||
Felipe, should this fix be uplifted to Aurora 47 in preparation for our e10s experiments on Beta 47?
Assignee | ||
Comment 20•8 years ago
|
||
Thanks for the ping, Chris. This one is conditional on bug 1257554, as both should ride together. I'll keep the needinfo here until there's an answer on bug 1257554.
Assignee | ||
Comment 21•8 years ago
|
||
Comment on attachment 8737300 [details] MozReview Request: Bug 1260460 - reset user's restore on demand preference during e10s migration. r=mconley Approval Request Comment [Feature/regressing bug #]: With the removal of the "Restore on Demand" checkbox from the Preferences UI (bug 1257554), we should also reset this pref to its default value, once, on migration. [User impact if declined]: Checkbox will be removed but pref will remain as it was before. The intention is to discourage users from using this. [Describe test coverage new/current, TreeHerder]: landed in central [Risks and why]: getting some users mad because their pref changed. But they can easily change it back through about:config [String/UUID change made/needed]: none
Flags: needinfo?(felipc)
Attachment #8737300 -
Flags: approval-mozilla-aurora?
Comment on attachment 8737300 [details] MozReview Request: Bug 1260460 - reset user's restore on demand preference during e10s migration. r=mconley Needed for e10s, Aurora47+
Attachment #8737300 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 23•8 years ago
|
||
Release Note Request (optional, but appreciated) [Why is this notable]: Some people want session restore to automatically load all their restored tabs, but this causes performance problems with e10s (bug 1037179). [Suggested wording]: "The browser.sessionstore.restore_on_demand preference has been reset to its default value (true)." or maybe include an explanation, though it makes the note longer: "The browser.sessionstore.restore_on_demand preference has been reset to its default value (true) to avoid e10s performance problems." [Links (documentation, blog post, etc)]: This bug?
relnote-firefox:
--- → ?
Assignee | ||
Comment 24•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/def03a483242
Comment 25•8 years ago
|
||
We try to avoid linking directly to bugzilla on release channel release notes and stick to posts or pages on mozilla.org domains. A post might still be nice. Would it be accurate to mark this "disabled" for 46 since e10s won't be on in 46 release? Will this affect non-e10s users on 46?
Flags: needinfo?(felipc)
Assignee | ||
Comment 26•8 years ago
|
||
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #25) > We try to avoid linking directly to bugzilla on release channel release > notes and stick to posts or pages on mozilla.org domains. A post might > still be nice. > > Would it be accurate to mark this "disabled" for 46 since e10s won't be on > in 46 release? Will this affect non-e10s users on 46? It affects all users, since the reset was meant to everyone. I agree that linking to bugzilla is not a good idea. A page on SUMO would be better. I tried to find a page documenting this feature ("Restore on demand"), where we could add a note saying that the pref was reset, but I didn't find anything..
Flags: needinfo?(felipc)
Comment 27•8 years ago
|
||
Joni is this something you could do in SUMO? Then I can link to it.
Flags: needinfo?(jsavage)
Added to Fx47 (beta) release notes.
Comment 29•8 years ago
|
||
Liz, yes, we can set up a SUMO page for this. I might need some additional info in order to write the article, since I'm not familiar with Restore on Demand: Can the average user get to it from the UI or is it only via about:config? Is "Restore on Demand" different from "Restore Previous session?" So if the user has E10s enabled, it will reset their about:config pref, so they have to go back in and change it? And it's only on 47 Beta, right? Not sure who to need info for answers, so I need info'ed Chris, but feel free to redirect to someone else.
Flags: needinfo?(jsavage) → needinfo?(cpeterson)
Comment 30•8 years ago
|
||
(In reply to Joni Savage ("need info" me) from comment #29) > I might need some additional info in order to write the article, since I'm > not familiar with Restore on Demand: Joni, if you have any more questions, feel free to keep asking in this bug or just ping me on IRC or email. > Can the average user get to it from the UI or is it only via about:config? The setting is available in Firefox 46 and earlier. In the about:preferences page, the user can uncheck the "Don’t load tabs until selected" checkbox so background tabs will load in the background instead of waiting for the user to click focus to the tab (the default setting). The "Don’t load tabs until selected" checkbox was removed in Firefox 47 (Beta). The feature is still available from the about:config pref "browser.sessionstore.restore_on_demand". > Is "Restore on Demand" different from "Restore Previous session?" "Restore Previous Session" will open placeholder tabs for all the tabs from your previous session. If "Restore on Demand" (aka the "Don’t load tabs until selected" checkbox) is enabled, then the placeholder tabs won't load their page contents until the user clicks focus to the tab. If "Restore on Demand" is disabled, then the placeholder tabs will slowly load their page contents in the background. This severely slows down Firefox because the user is trying to interact with the current tab, but Firefox is simultaneously loading all the background tabs' pages. > So if the user has E10s enabled, it will reset their about:config pref, so > they have to go back in and change it? And it's only on 47 Beta, right? The about:config pref "browser.sessionstore.restore_on_demand" will be reset for all users, whether they have e10s enabled or not, when the first run a version of Firefox >= 47.
Flags: needinfo?(cpeterson)
Comment 31•8 years ago
|
||
Thanks for the clarification, Chris. We've drafted an article here: https://support.mozilla.org/kb/allow-firefox-load-multiple-tabs-multiprocess If you have any changes, please let us know by commenting here, emailing me, or editing the article directly. One of our reviewers will take a look and publish it.
Assignee | ||
Comment 32•8 years ago
|
||
Thanks Joni for looking into it. It's looking good but I had some suggestions to improve it, which I have posted some as an edit and in a thread on the SUMO article.
Comment 33•8 years ago
|
||
Thanks for clarifying, Felipe. I understand now. I've published your changes and changed the title too: https://support.mozilla.org/en-US/kb/allow-firefox-load-multiple-tabs-background. The old link will redirect to this new one automatically, in case anyone has already shared it.
Comment 34•7 years ago
|
||
Quick question for anyone CC'd. Do you know if pre-loading tabs (when restore_on_demand = false), will perform better once e10s-multi (bug 1207306) is available?
Comment 35•7 years ago
|
||
(In reply to Chris More [:cmore] from comment #34) > Quick question for anyone CC'd. Do you know if pre-loading tabs (when > restore_on_demand = false), will perform better once e10s-multi (bug > 1207306) is available? e10s-multi should make the Firefox UI more responsive during the stampede of restoring many tabs, but it probably won't make the tab loading any faster.
Comment 36•7 years ago
|
||
(In reply to Chris Peterson [:cpeterson] from comment #35) > (In reply to Chris More [:cmore] from comment #34) > > Quick question for anyone CC'd. Do you know if pre-loading tabs (when > > restore_on_demand = false), will perform better once e10s-multi (bug > > 1207306) is available? > > e10s-multi should make the Firefox UI more responsive during the stampede of > restoring many tabs, but it probably won't make the tab loading any faster. Makes sense. When e10s-multi is released and loading tabs doesn't slow down the Firefox UI, would we consider bring back this feature?
Comment 37•7 years ago
|
||
(In reply to Chris More [:cmore] from comment #36) > Makes sense. When e10s-multi is released and loading tabs doesn't slow down > the Firefox UI, would we consider bring back this feature? I don't know. That's a question the Product team. :) I liked this feature and it is Chrome's behavior (when session restore is enabled), but given the startup performance problems and Dolkse's comments in bug 1257554 comment 6, I don't expect it to be prioritized.
You need to log in
before you can comment on or make changes to this bug.
Description
•