Closed Bug 1301016 Opened 3 years ago Closed 3 years ago

Pages in reader mode forget their scroll position across restarts

Categories

(Firefox for Android :: Reader View, defect)

49 Branch
All
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 51
Tracking Status
firefox48 --- unaffected
firefox49 + wontfix
firefox50 + verified
firefox51 --- verified

People

(Reporter: billiob, Assigned: JanH)

References

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160606114208

Steps to reproduce:

Open a page with some content in reader mode and scroll it a bit.
Close Firefox and launch it again.



Actual results:

The page is loaded in reader mode but it's scroll position is lost.


Expected results:

The page is loaded in reader mode and is scrolled to the last saved position
This is the same issue as https://bugzilla.mozilla.org/show_bug.cgi?id=1153393 but on Firefox for Android.

I think I saw it work in Firefox for Android 49.0b04 (or less) but at least since 49.0b09, it is not working.
I am not totally sure it worked in older beta versions; the bug may be there for long.
OS: Unspecified → Android
Hardware: Unspecified → ARM
Porting the desktop patch from bug 1153393 over to the mobile session store seems to work fine - I'll try and see whether I can adapt our mobile session store tests for this, too.
Assignee: nobody → jh+bugzilla
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hardware: ARM → All
See Also: → 1153393
Depends on: 1153393
See Also: 1153393
Blocks: 810981
Comment on attachment 8790108 [details]
Bug 1301016 - Part 1 - Wait for custom event instead of pageshow for scroll position restoring in reader mode.

https://reviewboard.mozilla.org/r/78046/#review76966
Attachment #8790108 - Flags: review?(ahunt) → review+
Comment on attachment 8790109 [details]
Bug 1301016 - Part 2 - Test scroll position restoring on reader mode pages.

https://reviewboard.mozilla.org/r/78048/#review76972

Looks good, and works for me too \o/
Attachment #8790109 - Flags: review?(ahunt) → review+
Comment on attachment 8790108 [details]
Bug 1301016 - Part 1 - Wait for custom event instead of pageshow for scroll position restoring in reader mode.

Approval Request Comment
[Feature/regressing bug #]: Mobile session store, bug 810981
[User impact if declined]: Scroll position for reader mode pages won't be restored when restoring Firefox or a closed/zombified tab
[Describe test coverage new/current, TreeHerder]: existing scroll position test extended to cover reader mode as well
[Risks and why]: Minimal - the patch is trivial and a straight port of the fix for the desktop session store from bug 1153393
[String/UUID change made/needed]: none

Since scroll position restoring on mobile only landed in 49, it would be nice to be able to ship it with this fixed, but since it's not a major bug, I can understand if it's already too late for that.
Attachment #8790108 - Flags: approval-mozilla-release?
Attachment #8790108 - Flags: approval-mozilla-beta?
Attachment #8790108 - Flags: approval-mozilla-aurora?
Comment on attachment 8790109 [details]
Bug 1301016 - Part 2 - Test scroll position restoring on reader mode pages.

see above
Attachment #8790109 - Flags: approval-mozilla-release?
Attachment #8790109 - Flags: approval-mozilla-beta?
Attachment #8790109 - Flags: approval-mozilla-aurora?
Too late to ship with 49, but we could still consider it for a 49 dot release. I'll track this, leave it as affected, and we can hopefully uplift to 50 and test/verify there.
Attachment #8790108 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Attachment #8790109 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/43e595bc39d5
Part 1 - Wait for custom event instead of pageshow for scroll position restoring in reader mode. r=ahunt
https://hg.mozilla.org/integration/autoland/rev/4e6337f61e6b
Part 2 - Test scroll position restoring on reader mode pages. r=ahunt
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/43e595bc39d5
https://hg.mozilla.org/mozilla-central/rev/4e6337f61e6b
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Verified as fixed using Sony Xperia Z5 on Nightly 51(2016-09-15).
Status: RESOLVED → VERIFIED
Comment on attachment 8790108 [details]
Bug 1301016 - Part 1 - Wait for custom event instead of pageshow for scroll position restoring in reader mode.

Fixes a recent regression and was verified on Nightly51, Aurora50+
Attachment #8790108 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8790109 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed using Sony Xperia Z5 on Aurora 50(2016-09-16).
Depends on: 1304168
Should we still want this for 49.x, the fixed line from bug 1304168 *must* be included as well.
On having another look here, I think this should ride with 50. The user impact of having to scroll down on a restored tab if they restart doesn't seem big enough to justify rushing this fix to release.
Attachment #8790108 - Flags: approval-mozilla-release? → approval-mozilla-release-
Attachment #8790109 - Flags: approval-mozilla-release? → approval-mozilla-release-
You need to log in before you can comment on or make changes to this bug.