Page MenuHomePhabricator

MobileFrontend should use sessionStorage for expandedSections
Closed, ResolvedPublic

Description

MobileFrontend uses localStorage to keep track of expandedSections indefinitely. This causes localStorage to grow.

Ideally this should use session storage as this information is only really needed inside a session.

https://developer.mozilla.org/en-US/docs/Web/API/Window/sessionStorage

QA steps

  1. Visit beta cluster page on a mobile device with headings e.g. https://en.m.wikipedia.beta.wmflabs.org/wiki/Albert_Einstein sections should be collapsible by default.
  2. Expand 3 sections
  3. Click a link in one of the 3 sections
  4. Hit the back button to go back to the Albert Einstein page.
  5. Expected: when going back the 3 sections you opened in step 2 should still be open.

QA Results - Beta

ACStatusDetails
1T253137#6765940

QA Results - Prod

ACStatusDetails
1T253137#6773376

Event Timeline

Ideally this should use session storage as this information is only really needed inside a session.

It's unfortunate that the Web Platform calls this "session storage". A more accurate name (based on the documentation) would be "tab storage".

If the expanded section feature only needs to store information on a single page view (with perhaps the the extra persistence of backward/forward and "re-open closed tab" navigations that re-create that same page view in the same tab), then sessionStorage is great. If not, then you probably don't want that.

In a nut shell: sessionStorage is not like a session cookie. There is no equivalent to session cookies in HTML5 Storage.

This causes localStorage to grow.

Note that the quota for sessionStorage generally isn't different or larger. Why does it grow? What's the ideal max size? Can it be limited?

the expanded section feature only needs to store information on a single page view (with perhaps the the extra persistence of backward/forward and "re-open closed tab" navigations that re-create that same page view in the same tab), then sessionStorage is great. If not, then you probably don't wan that.

the purpose of the feature is solely to restore back button behavior - so if a user clicks a link in a section and returns to the page they find themselves in the same position.

Note that the quota for sessionStorage generally isn't different or larger. Why does it grow? What's the ideal max size? Can it be limited?

We could limit it yes, but given the purpose is only for sessions, I'm not sure if there are any advantages to let it grow. Right now if I visit an article and then visit it again 3 months later the sections will remain open. That doesn't seem useful and is also inaccurate given those sections may have rearranged.

Awesome. That's quite literally a perfect fit for sessionStorage.

Jdlrobson triaged this task as Medium priority.May 19 2020, 8:51 PM
Jdlrobson moved this task from Incoming to Triaged but Future on the Readers-Web-Backlog board.

Hi @Jdlrobson ,
Can you tell me which extension I have to download to solve this issue?

Hi @Jdlrobson ,
Can you tell me which extension I have to download to solve this issue?

It's Extension:MobileFrontend, and source is here

Do I have to download the geodata , pageImages, Visual Editor extensions.

Because it is mention in link : https://www.mediawiki.org/wiki/Extension:MobileFrontend
that Get the most out of MobileFrontend by adding these optional compatible extensions.

I have replace the mw.storage with mw.storage.session in the toggler.js,
but then also it is showing the expandedSection in the localstorage, and nothing in the sessionStorage.

I have replace the mw.storage with mw.storage.session in the toggler.js,
but then also it is showing the expandedSection in the localstorage, and nothing in the sessionStorage.

I'm not 100% sure what you mean here. If it's still saving to localStorage then something is not quite right with your patch. Note, ideally this patch will remove any existing items in localStorage from the previous code.

This comment was removed by Yash4357.
This comment was removed by Yash4357.

Hi @Jdlrobson,
When I am doing git commit it is showing me this type of interface:

Screenshot from 2021-01-08 17-40-38.png (729×1 px, 226 KB)

That is the precommit. It is telling you that there are tests that are failing because of your changes. You will need to run git commit --no-verify to override.

Change 655102 had a related patch set uploaded (by Yash9265; owner: Yash9265):
[mediawiki/extensions/MobileFrontend@master] MobileFrontend should use sessionStorage for expandedSections

https://gerrit.wikimedia.org/r/655102

Jdlrobson updated the task description. (Show Details)
Jdlrobson added a subscriber: Yash4357.

Change 655102 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] MobileFrontend should use sessionStorage for expandedSections

https://gerrit.wikimedia.org/r/655102

Test Result - Beta

Status: ✅ PASS
Environment: beta
OS: iOS 14.2.1
Browser: Safari
Device: iPhone 12 Max Pro
Emulated Device: NA

Test Artifact(s):

QA Steps

Visit beta cluster page on a mobile device with headings e.g. https://en.m.wikipedia.beta.wmflabs.org/wiki/Albert_Einstein sections should be collapsible by default.
Expand 3 sections
Click a link in one of the 3 sections
Hit the back button to go back to the Albert Einstein page.
AC1: Expected: when going back the 3 sections you opened in step 2 should still be open.

ezgif.com-gif-maker (1).gif (1×666 px, 3 MB)

Edtadros added a subscriber: Edtadros.

Test Result - Prod

Status: ✅ PASS
Environment: enwiki
OS: iOS 14.2.1
Browser: Chrome
Device: iPhone 11 Max Pro
Emulated Device: NA

Test Artifact(s):

QA Steps

Visit page on a mobile device with headings e.g. https://en.m.wikipedia.org/wiki/Albert_Einstein sections should be collapsible by default.
Expand 3 sections
Click a link in one of the 3 sections
Hit the back button to go back to the Albert Einstein page.
AC1: Expected: when going back the 3 sections you opened in step 2 should still be open.

Screen Recording 2021-01-25 at 6.12.29 AM.mov.gif (896×414 px, 1 MB)