Closed Bug 810981 Opened 7 years ago Closed 4 years ago

Restore scroll position when restoring tabs from previous session

Categories

(Firefox for Android :: Toolbar, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 49
Tracking Status
firefox49 --- fixed
relnote-firefox --- 49+
fennec + ---

People

(Reporter: kats, Assigned: JanH, Mentored)

References

Details

(Whiteboard: [lang=js])

Attachments

(7 files)

Taras reports: if you're reading a long article in Fennec, and then switch apps and do something so fennec gets oom-killed, switching back into fennec takes you back to the top of the page after the page is session-restored. We should save the scroll position in the session restore data and use it to scroll back to where the user was.
I was using reader mode. I think we should serialize the reader mode view when app gets backgrounded, so it doesn't have to get reloaded from network.
There is a more general issue with scroll position not being restored from session storage.

STR 1
1. Enable tabs restoring.
2. Open a page and scroll on it.
3. Close Fennec using the Android task manager.
4. Launch Fennec again.

STR 2
1. Open a page and scroll on it.
2. Change into guest mode (Tools/New Guest Session).
3. Exit guest session.

In both cases the following holds.

Expected
Tabs are restored with pages scrolled to previous positions.

Actual
Tabs are restored with scroll position reset to top left.

Do we want to track this in this bug and change the summary or spin off a new bug?
Flags: needinfo?(bugmail.mozilla)
I'm fine repurposing this bug to track the general issue.
Flags: needinfo?(bugmail.mozilla)
Summary: Restore scroll position after fennec gets oom killed → Restore scroll position when restoring tabs from previous session
Depends on: 697858
Status: ASSIGNED → NEW
* This could make us a better on-the-go article reader
* Due to the different interaction model, desktop does not have this problem.
* We should match user expectations – Android arbitrarily kills things and this (presumably) does not match the user's model of how Firefox is supposed to work.
tracking-fennec: --- → ?
Mentor: snorp
tracking-fennec: ? → +
Whiteboard: [lang=js]
Mentor: esawin
Duplicate of this bug: 999382
Eugen, I might dig into this at some point. Do you have any suggestions on where to get started?
Flags: needinfo?(esawin)
As Margaret said, SessionStore.js is a good place to start. Although, I'm not particularly helpful on this as a mentor as I've only worked on zoom level sessions which do not use SessionStore.js.
Flags: needinfo?(esawin)
I hope this doesn't lead too far down a rabbit hole into the C++ side of things, but in any case I'll try looking at it.
Assignee: nobody → jh+bugzilla
Depends on: 1265818
It seems that both we and Desktop have code to both serialise and deserialise the scroll position of a nsISHEntry, however for unknown reasons, nsISHEntry.getScrollPosition() always returns (0, 0), so scroll positions for history entries are never saved.
At a first glance, desktop works around this by saving the scroll position of the currently loaded page as a property of the tab object, similar to how we do form data saving.
I'll probably look at doing something similar for us, too.
(In reply to Jan Henning [:JanH] from comment #10)
> It seems that both we and Desktop have code to both serialise and
> deserialise the scroll position of a nsISHEntry, however for unknown
> reasons, nsISHEntry.getScrollPosition() always returns (0, 0), so scroll
> positions for history entries are never saved.

Maybe kats or someone else from the platform team can help explain what's going on here.
Flags: needinfo?(bugmail.mozilla)
Off the top of my head I'm not sure. From a quick look around the code I *believe* the scroll position is saved in the mOSHE->SetScrollPosition calls in docshell/base/nsDocShell.cpp, but I see only two calls to that function, one for pushState/replaceState and one for "short-circuit loads" which appear to be in-document only. So I don't know that the scroll position is saved in the session history across page navigation.

There's also code in nsGfxScrollFrame::SaveState/RestoreState which saves and restores the scroll position when the frame tree is rebuilt, but that also shouldn't happen across page navigations unless it's in the bfcache. Again, not 100% sure on this.

Based on comment 10 I think doing what desktop does and saving it in the tab object sounds reasonable. Feel free to needinfo me again if you want me to look at this in greater detail, but it probably won't happen this week.
Flags: needinfo?(bugmail.mozilla)
Just a quick status update, I have implemented some basic scroll position and zoom level capturing and restoring code, which seems to be working quite well.
I'm still looking on ironing out a few rough edges - scroll position restoring doesn't seem to be working for pages with framesets yet, and part of the zoom level restoring could benefit from some additional support by the PresShell and/or the MobileViewportManager, because the session store doesn't know anything about how to adjust the zoom level/resolution if the screen orientation has changed between data capturing and restoring.
What is the expected behaviour if the screen orientation changed between capture/restore? Should it behave the same as if you restored without rotating, and then rotated the phone? Or do you want to restore to the resolution that was captured, even if it doesn't fit quite right?
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #14)
> What is the expected behaviour if the screen orientation changed between
> capture/restore? Should it behave the same as if you restored without
> rotating, and then rotated the phone?

Exactly, that would be my desired behaviour.

> Or do you want to restore to the
> resolution that was captured, even if it doesn't fit quite right?

This is what is currently happening with my code. As far as I can see, this is not a problem for pages where the viewport scales to the device width (i.e. probably most/all? mobile pages, if I'm understanding that right), because there the reported resolution [1] doesn't seem to change on rotation, but for desktop pages for example, the resolution is adjusted when the screen width changes upon rotation.

With my current approach, a desktop page captured in portrait mode and restored in landscape mode might e.g. end up looking like in the attached screenshot.

I'm not terribly familiar with the native code, so there might be a better approach to solve this, but this is what I was thinking about so far:
- As a minimal solution, an expanded version of setResolutionAndScaleTo(), which would also take the previous screen dimensions as a parameter. If the previous screen dimension matches the current one, it would be equivalent to a simple call to setResolutionAndScaleTo(aResolution), if not, it would recalculate the resolution based on the old and new screen dimension before scaling the page.
- Ideally, some way of getting the session store's data about the old screen size and zoom level (and possibly the scroll position as well?) into the PresShell/MobileDeviceManager before the first paint/load event, so that SetInitialViewport() could just directly scale the page to the correct level. Otherwise, the MobileDeviceManager would end up scaling the page to its default initial level on first-paint/load, only to be immediately overridden by the session store's call to setResolutionAndScaleTo during pageshow.

[1] I'm currently using nsIDOMWindowUtils.getResolution() to capture the zoom level.
(In reply to Jan Henning [:JanH] from comment #15)
> - As a minimal solution, an expanded version of setResolutionAndScaleTo(),
> which would also take the previous screen dimensions as a parameter. If the
> previous screen dimension matches the current one, it would be equivalent to
> a simple call to setResolutionAndScaleTo(aResolution), if not, it would
> recalculate the resolution based on the old and new screen dimension before
> scaling the page.

This code to "recalculate the resolution" is pretty straightforward, it is at [1]. If you have the old and new screen dimensions in your restore code, you could probably just adjust the captured resolution by the same ratio before you restore it via SetResolutionAndScaleTo. That might be a little cleaner than trying to plumb it in through a new expanded version of setResolutionAndScaleTo, although I don't have all this code paged into my head right now so I might be missing something.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/MobileViewportManager.cpp?rev=8fa535b41cad#211

> - Ideally, some way of getting the session store's data about the old screen
> size and zoom level (and possibly the scroll position as well?) into the
> PresShell/MobileDeviceManager before the first paint/load event, so that
> SetInitialViewport() could just directly scale the page to the correct
> level. Otherwise, the MobileDeviceManager would end up scaling the page to
> its default initial level on first-paint/load, only to be immediately
> overridden by the session store's call to setResolutionAndScaleTo during
> pageshow.

We could add something for this, yeah. We could set a desired initial resolution in the MobileViewportManager and the mIsFirstPaint branch of the UpdateResolution function could use that resolution instead of the default zoom that it picks up from the viewport info. It would probably still need to clamp that provided resolution by the min/max zooms in the viewport info.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #16)
> This code to "recalculate the resolution" is pretty straightforward, it is
> at [1]. If you have the old and new screen dimensions in your restore code,
> you could probably just adjust the captured resolution by the same ratio
> before you restore it via SetResolutionAndScaleTo. That might be a little
> cleaner than trying to plumb it in through a new expanded version of
> setResolutionAndScaleTo, although I don't have all this code paged into my
> head right now so I might be missing something.

I was trying to avoid duplicating that code, but you're right, feeding a fake previous display size and zoom level to the MobileViewportManager doesn't seem super-trivial, either. Also, the session store wouldn't have to calculate a zoom level from scratch, but just adjust a pre-existing one, so the calculation should indeed be quite simple. I hope that looking for autoSize in getViewportInfo (https://dxr.mozilla.org/mozilla-central/source/dom/interfaces/base/nsIDOMWindowUtils.idl#118) is enough to catch those cases where we *don't* want to adjust the zoom level after a display orientation change. I guess I'll just give it a try and see whether I can get it working satisfactorily.

> > - Ideally, some way of getting the session store's data about the old screen
> > size and zoom level (and possibly the scroll position as well?) into the
> > PresShell/MobileDeviceManager before the first paint/load event, so that
> > SetInitialViewport() could just directly scale the page to the correct
> > level. Otherwise, the MobileDeviceManager would end up scaling the page to
> > its default initial level on first-paint/load, only to be immediately
> > overridden by the session store's call to setResolutionAndScaleTo during
> > pageshow.
> 
> We could add something for this, yeah. We could set a desired initial
> resolution in the MobileViewportManager and the mIsFirstPaint branch of the
> UpdateResolution function could use that resolution instead of the default
> zoom that it picks up from the viewport info.

That would be nice.

> It would probably still need
> to clamp that provided resolution by the min/max zooms in the viewport info.

Does this account for the user option to override any meta viewport zoom restrictions? Besides, the session store would capture the resolution as it actually existed, so as long as that value is suitably recalculated to account for any display size/orientation changes, there shouldn't be any harm in restoring it "as is". But we can just try this out anyway when implementing it...
Yes, it should account for the user override. That gets applied at [1] so the values in the viewport info will already be wider by the time MobileViewportManager gets it.

[1] http://mxr.mozilla.org/mozilla-central/source/dom/base/nsDocument.cpp?rev=85ce8cb0639a#8108
Getting scroll position restoring to work on pages with frames was pretty straightforward - just move the top-level event filtering code I've removed in bug 1269189 to the load/pageshow events, so I'm restoring the scroll position only once all subframes have loaded and voilà, it's working.

There's still something flaky about restoring the zoom level for pages with framesets, though:
- If I'm disabling scroll position restoring, calling setResolutionAndScaleTo() when receiving the top level document's pageshow event doesn't seem to have any effect
- If I'm also restoring the scroll positions after calling setResolutionAndScaleTo(), there's something weird going on. The resolution change is still being ignored, but for unfathomable reasons I end up with a viewport that's covering only part of my display size.
After closing the page and restoring it again - my session store code is calling setResolutionAndScaleTo and then ScrollPosition.jsm's restoreTree() when receiving the top-level pageshow event from the browser.
Depends on: 1270019
Could you post the patches you have so far? I'm having trouble following your description of the changes and it would probably be easier to just look at the code. Even just a github link or try push is fine.
I applied your patches and ran them. Your restoreZoom function is doing the right thing, but the problem is that the C++ APZ code is maintaining the "visual zoom level" rather than updating the visual zoom from what you specify. I think if we fix bug 1270019 that will take care of this, because if the zoom is set before the APZ gets the first-paint notification, then it will use whatever zoom is provided to it.
This copies the approach we've taken for form data saving and applies it to recording the current scroll position of the page, too.

This means that after receiving a scroll event, we capture the scroll position for the top level document and all direct child frames and include it in the session store data. Because compared to the form data input events the scroll event can fire at a relatively high rate, we throttle the scroll position capturing using timeouts to run at most twice per second.

Review commit: https://reviewboard.mozilla.org/r/53736/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/53736/
Attachment #8754136 - Flags: review?(margaret.leibovic)
Attachment #8754137 - Flags: review?(margaret.leibovic)
Attachment #8754138 - Flags: review?(margaret.leibovic)
Attachment #8754139 - Flags: review?(margaret.leibovic)
Since we're now recording the scroll position in the session data, it makes sense to store the zoom level as well.

To do this, we make use of the new facility introduced in bug 1270019, which allows us to provide a desired initial resolution to the MobileViewportManager. For this to work, we need to send our desired zoom level before the MVM calculates the initial page resolution, i.e. before first paint/page load. Therefore, we now have browser.js notify us of location change events, so we can set the zoom level to restore early enough.

This also means that we can no longer restore the scroll position on load, because the MobileViewportManager applies the resolution we provide it via utils.setRestoreResolution() only after the first paint or page load ( whichever happens earlier). In the latter case, our JS load event handler will run shortly before the MVM has applied the desired zoom level in its own event handling code, which means that any scroll attempt which depends on the page already being zoomed in will fail to apply.
Therefore, the scroll position restoring needs to be moved to a later point in time, i.e. in this case the "pageshow" event.

Review commit: https://reviewboard.mozilla.org/r/53738/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/53738/
On pages that aren't "width=device-width" or similar, Gecko adjusts the resolution when the display dimensions change, e.g. because the device has been rotated. The session store needs to do something similar when restoring a page if the device orientation has changed since the moment the tab state was captured.

Therefore, we now include the width of the browser window in the saved zoom data and use it to scale the zoom level as necessary when restoring a tab.

Review commit: https://reviewboard.mozilla.org/r/53740/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/53740/
Comment on attachment 8754136 [details]
MozReview Request: Bug 810981 - Part 1 - Record current scroll position in mobile session store. r=margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53736/diff/1-2/
Comment on attachment 8754137 [details]
MozReview Request: Bug 810981 - Part 2 - Save the current zoom level when scrolling. r=kats r=margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53738/diff/1-2/
Comment on attachment 8754138 [details]
MozReview Request: Bug 810981 - Part 3 - Recalculate zoom level before restoring to a different screen orientation. r=kats

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53740/diff/1-2/
Comment on attachment 8754139 [details]
MozReview Request: Bug 810981 - Part 4 - Test session store scroll position and zoom level handling. r=margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53742/diff/1-2/
Attachment #8754136 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8754136 [details]
MozReview Request: Bug 810981 - Part 1 - Record current scroll position in mobile session store. r=margaret

https://reviewboard.mozilla.org/r/53736/#review52282

Nice.

::: mobile/android/components/SessionStore.js:320
(Diff revision 2)
> +        if (!this._scrollSavePending) {
> +          this._scrollSavePending =
> +            window.setTimeout((function(window, browser) {
> +              this._scrollSavePending = null;
> +              this.onTabScroll(window, browser);
> +            }).bind(this), 500, window, browser);

If you want to avoid the bind call and passing extra paramaeters, you can use an arrow function here:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions
Attachment #8754137 - Flags: review?(margaret.leibovic)
Comment on attachment 8754137 [details]
MozReview Request: Bug 810981 - Part 2 - Save the current zoom level when scrolling. r=kats r=margaret

https://reviewboard.mozilla.org/r/53738/#review52292

I think you should get kats to review this change, since I'm not as familiar with the zoom logic details.

::: mobile/android/components/SessionStore.js:194
(Diff revision 2)
>          }
>          break;
>        }
> +      case "Session:RestoreZoom": {
> +        let window = Services.wm.getMostRecentWindow("navigator:browser");
> +        let browser = window.BrowserApp.getTabForId(aData).browser;

Can we send the browser as the notification data, instead of just the tab id? Then we wouldn't need to get the window here.
Attachment #8754137 - Flags: review?(bugmail.mozilla)
Comment on attachment 8754138 [details]
MozReview Request: Bug 810981 - Part 3 - Recalculate zoom level before restoring to a different screen orientation. r=kats

I also think kats would be a better reviewer for this.
Attachment #8754138 - Flags: review?(margaret.leibovic) → review?(bugmail.mozilla)
Comment on attachment 8754139 [details]
MozReview Request: Bug 810981 - Part 4 - Test session store scroll position and zoom level handling. r=margaret

https://reviewboard.mozilla.org/r/53742/#review52298

Nice tests!
Attachment #8754139 - Flags: review?(margaret.leibovic) → review+
Comment on attachment 8754137 [details]
MozReview Request: Bug 810981 - Part 2 - Save the current zoom level when scrolling. r=kats r=margaret

https://reviewboard.mozilla.org/r/53738/#review52316

The zoom-related bits look fine to me, although I don't trust myself to spot any flaws in the overall save/restore architecture.
Attachment #8754137 - Flags: review?(bugmail.mozilla) → review+
Attachment #8754138 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8754138 [details]
MozReview Request: Bug 810981 - Part 3 - Recalculate zoom level before restoring to a different screen orientation. r=kats

https://reviewboard.mozilla.org/r/53740/#review52318

::: mobile/android/components/SessionStore.js:1339
(Diff revision 2)
>    * Restores the zoom level of the window. This needs to be called before
>    * first paint/load (whichever comes first) to take any effect.
>    */
>    _restoreZoom: function ss_restoreZoom(aScrollData, aBrowser) {
>      if (aScrollData && aScrollData.zoom) {
> +      aScrollData.zoom.resolution = this._recalculateZoom(aScrollData.zoom);

I feel it would be better to use a local variable rather than clobbering the existing value in-place, just in case this function is called multiple times on the same object.
Comment on attachment 8754136 [details]
MozReview Request: Bug 810981 - Part 1 - Record current scroll position in mobile session store. r=margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53736/diff/2-3/
Attachment #8754137 - Attachment description: MozReview Request: Bug 810981 - Part 2 - Save the current zoom level when scrolling. r=margaret → MozReview Request: Bug 810981 - Part 2 - Save the current zoom level when scrolling. r=kats r=margaret
Attachment #8754138 - Attachment description: MozReview Request: Bug 810981 - Part 3 - Recalculate zoom level before restoring to a different screen orientation. r=margaret → MozReview Request: Bug 810981 - Part 3 - Recalculate zoom level before restoring to a different screen orientation. r=kats
Attachment #8754137 - Flags: review?(margaret.leibovic)
Comment on attachment 8754137 [details]
MozReview Request: Bug 810981 - Part 2 - Save the current zoom level when scrolling. r=kats r=margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53738/diff/2-3/
Comment on attachment 8754138 [details]
MozReview Request: Bug 810981 - Part 3 - Recalculate zoom level before restoring to a different screen orientation. r=kats

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53740/diff/2-3/
Comment on attachment 8754139 [details]
MozReview Request: Bug 810981 - Part 4 - Test session store scroll position and zoom level handling. r=margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53742/diff/2-3/
https://reviewboard.mozilla.org/r/53738/#review52292

> Can we send the browser as the notification data, instead of just the tab id? Then we wouldn't need to get the window here.

Good point, the browser can be sent via the `aSubject`.
https://reviewboard.mozilla.org/r/53736/#review52282

> If you want to avoid the bind call and passing extra paramaeters, you can use an arrow function here:
> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions

Seems the MDN documentation on setTimeout could do with a bit of updating... thanks for pointing this out.
https://reviewboard.mozilla.org/r/53740/#review52318

> I feel it would be better to use a local variable rather than clobbering the existing value in-place, just in case this function is called multiple times on the same object.

Good point. That function should only ever be called once, but if a session save is triggered before `onTabScroll()` had a chance to run again, the stored zoom level and display width would be out of sync with each other.
Comment on attachment 8754137 [details]
MozReview Request: Bug 810981 - Part 2 - Save the current zoom level when scrolling. r=kats r=margaret

https://reviewboard.mozilla.org/r/53738/#review52906

I'm having a tough time following this session restore logic myself. This seems to fit with our existing patterns, but I want to make sure we're not adding more complexity for the sake of going along with what's already in place.

::: mobile/android/chrome/content/browser.js:4471
(Diff revision 3)
>      if (!sameDocument) {
>        // XXX This code assumes that this is the earliest hook we have at which
>        // browser.contentDocument is changed to the new document we're loading
>        this.contentDocumentIsDisplayed = false;
>        this.hasTouchListener = false;
> +      Services.obs.notifyObservers(this.browser, "Session:RestoreZoom", null);

Why do we need this notification, separately from the "Session:Restore" notification? Is it because we always restore the zoom on every location change, not just during session restore?

If I follow the logic below correctly, it seems like we only do the actual restore during session restore, though.

::: mobile/android/components/SessionStore.js:195
(Diff revision 3)
>          break;
>        }
> +      case "Session:RestoreZoom": {
> +        let browser = aSubject;
> +        if (browser.__SS_restoreDataOnLocationChange) {
> +          delete browser.__SS_restoreDataOnLocationChange;

It's confusing that this flag mentions location change, but the "Session:RestoreZoom" notification is not obviously coupled to location change.

Could you add some comments to clarify what's going on here?

::: mobile/android/components/SessionStore.js:1258
(Diff revision 3)
>  
>      // Restoring text data and scroll position requires waiting for the content
>      // to load. So we set a flag and delay this until the appropriate event.
> +    aBrowser.__SS_restoreDataOnLocationChange = true;
>      aBrowser.__SS_restoreDataOnLoad = true;
> +    aBrowser.__SS_restoreDataOnPageshow = true;

It feels a bit fragile to continue adding more of these flags. You should also update the comment above to explain why these are necessary and what their intended purpose is.
Attachment #8754137 - Flags: review?(margaret.leibovic) → review+
https://reviewboard.mozilla.org/r/53738/#review52906

> Why do we need this notification, separately from the "Session:Restore" notification? Is it because we always restore the zoom on every location change, not just during session restore?
> 
> If I follow the logic below correctly, it seems like we only do the actual restore during session restore, though.

`Session:Restore` is only sent during app startup if we are automatically restoring last session's tabs (the tabs restore setting set to "Always restore").

> If I follow the logic below correctly, it seems like we only do the actual restore during session restore, though.

Yes, but there are other occasions, too, where we are restoring tabs with their history apart from the full session restore on startup:
- The Recent Tabs panel
- The "Undo close tab" snackbar

To expand on this, when a document loads, the [`MobileViewportManager`](https://dxr.mozilla.org/mozilla-central/source/layout/base/MobileViewportManager.cpp) set the document's initial resolution on either first paint or load, whichever happens earlier.
My original implementation then called `setResolutionAndScaleTo()` after pageshow to set the restored zoom level, however that had some drawbacks:
- After Gecko had probably already started painting the document at the default resolution, we'd then override that with a new zoom level and would have to paint everything again.
- It gave some strange results and didn't really work on pages with frames.

So instead, kats thankfully provided an API to set a desired initial resolution, which would be taken into account by the `MobileViewportManager` during its initial resolutin calculation. The caveat with that is that we *have* to call that API before this initial resolution calculation happens (as mentioned above, either on load or on first paint), otherwise it won't have any effect.
Unfortunately calling this directly after restoring the history in `_restoreTab()` seems to be too early, so the earliest opportunity we have is when we receive a location change event.

And finally, piggybacking on browser.js's web progress listeners in order to be notified about location change events seemed the easiest option here.

> It's confusing that this flag mentions location change, but the "Session:RestoreZoom" notification is not obviously coupled to location change.
> 
> Could you add some comments to clarify what's going on here?

Alternatively, I could rename the notification to something like "Session:NotifyLocationChange"?

> It feels a bit fragile to continue adding more of these flags. You should also update the comment above to explain why these are necessary and what their intended purpose is.

Aye aye.
Ideally we'd be able to do all of these things directly in `_restoreTab()`, but in practice unfortunately a number of things can only be done after tab loading has progressed far enough, with "far enough" also depending on what exactly we want to do.
I'll add separate comments for each of these flags, though.
Comment on attachment 8754136 [details]
MozReview Request: Bug 810981 - Part 1 - Record current scroll position in mobile session store. r=margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53736/diff/3-4/
Comment on attachment 8754137 [details]
MozReview Request: Bug 810981 - Part 2 - Save the current zoom level when scrolling. r=kats r=margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53738/diff/3-4/
Comment on attachment 8754138 [details]
MozReview Request: Bug 810981 - Part 3 - Recalculate zoom level before restoring to a different screen orientation. r=kats

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53740/diff/3-4/
Comment on attachment 8754139 [details]
MozReview Request: Bug 810981 - Part 4 - Test session store scroll position and zoom level handling. r=margaret

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53742/diff/3-4/
Release Note Request (optional, but appreciated)
[Why is this notable]: Tab zombification (when Firefox unloads a background tab under memory pressure) no longer means loosing your scroll position. Also works for closing and restoring a tab and closing and reopening Firefox as a whole.
[Suggested wording]: Remember scroll position and zoom level for open tabs.
[Links (documentation, blog post, etc)]: none
relnote-firefox: --- → ?
Depends on: 1279443
Depends on: 1282902
Depends on: 1301016
You need to log in before you can comment on or make changes to this bug.