Page MenuHomePhabricator

[Bug] No failure states for various workflows
Open, NormalPublic

Description

Various components in mobile load code asynchronously. When you go offline, this code fails to load. We will introduce a generic error state for these situations.

This error state will show for the following workflows when loaded offline:

  • editor
  • talk
  • mediaviewer
  • category
  • languages
  • editor

Environments Observed

Browser Version

  • Chromium v68.0.3440.75

OS Version

  • Ubuntu v18.04

Device Model

  • Desktop

Device Language

  • English

Related to T193172 but appears to be distinct.

Design

Perhaps we could introduce a generic "no internet connection" error here (assuming we know specifically what the issue is)? From there the user can "X" out of the editor. In terms of how long the timeout should be, how long does it take to detect? I think waiting for ~3-4 seconds would be fine.

Developer notes

This is actually a wide-spread problem. The LoadingOverlay has no fallbacko or timeout.
The LoadingOverlay is usually used resources/mobile.startup/rlModuleLoader.js and resources/mobile.special.nearby.scripts/nearby.js and resources/skins.minerva.talk/init.js

The same issue can be replicated for language overlay, category overlay and various other screens.

The EditorOverlay is a little more complicated as it isn't using the LoadingOverlay (when it should be) but if we had a generic mechanism for async loading we could use that.

Acceptance criteria

  • A new generic "no internet connection" MessageBox error will be shown in the body of impacted overlays when a HTTP request fails
  • The error state is shown on Nearby, Talk, MediaViewer
  • Remove the rlModuleLoader code from MobileFrontend - use OverlayManager.replaceCurrent instead
  • Workflows in QA step all show error feedback to the user

QA steps

Steps to Reproduce problem with talk

  1. Visit https://en.m.wikipedia.org/wiki/User:Jdlrobson
  2. Go offline
  3. Tap talk icon

The page cannot be interacted with and the action cannot be cancelled. It never times out.

Steps to Reproduce problem with media viewer

  1. In incognito window visit https://en.m.wikipedia.org/wiki/Spain
  2. Go offline
  3. Click an image

The page cannot be interacted with and the action cannot be cancelled. It never times out.

Steps to Reproduce problem with languages

  1. visit https://en.m.wikipedia.org/wiki/Spain
  2. Go offline
  3. Click the language icon.

No feedback is given to the user.

Steps to Reproduce problem with category

  1. In beta while logged in visit https://en.m.wikipedia.org/wiki/Spain
  2. Go offline
  3. Scroll to bottom of page and tap "categories"

The page cannot be interacted with and the action cannot be cancelled. It never times out.

Steps to Reproduce problem with Editing

  1. Visit https://en.m.wikipedia.org/wiki/Barack_Obama
  2. Go offline
  3. Tap the edit pencil

Expected Results

  • An error message is presented to inform the user that loading the editor failed

Actual Results

This header shows and the spinner never stops

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 10 2018, 2:12 PM
Jdlrobson renamed this task from [Bug] When loading the edit overlay fails, a translucent sheet occludes the page and disables interactivity to [Bug] Async loading code if failing occludes the page and disables interactivity.Aug 10 2018, 4:56 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson added subscribers: alexhollender, Jdlrobson.

@alexhollender need some guidance on what to do here
Specifically

  • how long should a timeout be?
  • can user dismiss the loading event (if so via what control? toast? clicking multiple times?)
  • What should failure look like (toast saying "Failed to load workflow. Please try again.")
Jdlrobson updated the task description. (Show Details)Aug 10 2018, 4:58 PM
Jdlrobson updated the task description. (Show Details)

I followed the steps to reproduce, upon clicking "Edit" I land in the editor and get a spinner indefinitely.

Perhaps we could introduce a generic "no internet connection" error here (assuming we know specifically what the issue is)? From there the user can "X" out of the editor. In terms of how long the timeout should be, how long does it take to detect? I think waiting for ~3-4 seconds would be fine.

Restricted Application changed the subtype of this task from "Deadline" to "Task". · View Herald TranscriptSep 17 2018, 3:07 PM

@alexhollender, looks great to me. We can show this if the HTTP request fails for whatever reason. When truly offline, requests fail instantly on my device but it may take some time on a flaky connection, just like it does when loading a normal page.

Jdlrobson added a comment.EditedSep 24 2018, 8:27 PM

Not ready for development just yet!

This issue is a little broader. Usually this spinner is loading the code in order to load the code to display the overlay. I the case of the media viewer and editor we cannot render the editor or image overlay until the code has loaded and we dont know at that point whether the overlay is a black overlay or white one.

Ideally the treatment should NOT be tied to the overlay design or should be generic enough to work regardless of how it eventually loads. If we keep the current mock you've proposed I am sure we can find a solution, but will require some additional thinking.

It might be worth thinking about how this would look in the media viewer case and Nearby case (which doesnt have an overlay) and we can iterate from there.

Ideally I'd want a generic solution not tied to the target overlay (is there an overlay we could show specifically for dropped connections/loading errors?).

Jdlrobson updated the task description. (Show Details)Sep 24 2018, 8:42 PM

I've updated description with added replication steps to show how this is not limited to the edit overlay and how we should consider thinking of this more generically.

Change 486385 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] Add #loadModuleAndGetOverlay: a nicer way to use LoadingOverlay

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

Change 486386 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] Improve loading screens by using #loadModuleAndGetOverlay

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

Change 486387 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] Improve loading screens by using #loadModuleAndGetOverlay

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

Change 486388 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] LoadingOverlay: Set translucency using rgba() color instead of opacity

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

I investigated this since it was one of the issues we faced in work on VisualEditor overlay: T210630: [Engineering EPIC]: Loading screen improvements

Note that this implements only a few parts of the proposed fix:

Generalise the code in M.require( 'mobile.startup/rlModuleLoader' ) so it can be used for asynchronously loading anything async, regardless of where it comes from (e.g. not RL specific). rlModuleLoader should make use of the generic async code

Done in https://gerrit.wikimedia.org/r/486385 "Add #loadModuleAndGetOverlay: a nicer way to use LoadingOverlay"

This does not implement any new error handling and timeout behaviour. However, it allows the user to get "unstuck" by pressing the back button in their browser. And I think it will be easier to further extend with these features than the current code.

Update Talk in MobileFrontend to use the new asyncLoader

Done in https://gerrit.wikimedia.org/r/486387 [MinervaNeue] "Improve loading screens by using #loadModuleAndGetOverlay" (the overlay's code has been moved to Minervaneue since this was filed?)

This also handles the categories, media and languages overlays.

Create task to update Editor in MobileFrontend to be rewritten to use the new asyncLoader

Actually, I just implemented it.

Done in https://gerrit.wikimedia.org/r/486386 [MobileFrontend] "Improve loading screens by using #loadModuleAndGetOverlay"

This does not touch Nearby code.

There is a loading spinner that persists but I never noticed it prior to taking screenshots since it has very low contrast. If you don't notice it, the page appears to be in a disabled state.

Patch https://gerrit.wikimedia.org/r/486388 "LoadingOverlay: Set translucency using rgba() color instead of opacity" improves this.

Change 486385 abandoned by Bartosz Dziewoński:
Add #loadModuleAndGetOverlay: a nicer way to use LoadingOverlay

Reason:
Jon is working on an alternative solution as part of T212465.

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

Change 486386 abandoned by Bartosz Dziewoński:
Improve loading screens by using #loadModuleAndGetOverlay

Reason:
Jon is working on an alternative solution as part of T212465.

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

Change 486387 abandoned by Bartosz Dziewoński:
Improve loading screens by using #loadModuleAndGetOverlay

Reason:
Jon is working on an alternative solution as part of T212465.

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

Change 486388 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] LoadingOverlay: Set translucency using rgba() color instead of opacity

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

What is the problem we're left to solve now? From the look of it everything looks fixed to me. @alexhollender was there anything specific you wanted to do here?

What is the problem we're left to solve now? From the look of it everything looks fixed to me. @alexhollender was there anything specific you wanted to do here?

I am still able to reproduce the issue so I assumed this still needs to be done.

Jdlrobson renamed this task from [Bug] Async loading code if failing occludes the page and disables interactivity to [Bug] No failure states for various workflows.Tue, Sep 17, 7:44 PM
Jdlrobson updated the task description. (Show Details)

@alexhollender looks like the media viewer already has an error state:
https://doc.wikimedia.org/MobileFrontend/master/js/ui/?path=/story/mediaviewer--loaderrormessage

Shall I pull this into its own task?

matmarex removed a subscriber: matmarex.Tue, Sep 17, 8:58 PM