Page MenuHomePhabricator

Red links displayed as regular links when Restbase enabled.
Closed, ResolvedPublic2 Estimated Story Points

Description

Description

When Restbase is enabled, red links in an article are displayed as blue, and produce 404 errors when clicked.

Steps to reproduce

  1. Search for 'Staraya Ladoga'
  2. go to 'Sights and landmarks' - in the third paragraph there is a blue link 'Yelena River'
  3. click on the link - the app either crashes or displays 'This page does not exist'

Expected results

  • 'Yelena River' should not be a link, and should not be clickable.

Actual results

The app crashes or displays a page 'This page does not exist' :

Screenshot_2015-11-20-15-57-33.png (1×1 px, 71 KB)

Screenshot_2015-11-20-15-57-49.png (1×1 px, 71 KB)

Environments observed

App version: 2.1.134-alpha-2015-11-20
Android OS versions: Android 5.1.1
Device model: Nexus 5
Device language: English

Related Objects

StatusSubtypeAssignedTask
OpenFeatureNone
ResolvedJdlrobson
Resolved Mholloway
ResolvedNone
ResolvedArlolra
ResolvedEsanders
ResolvedCatrope
DeclinedNone
DeclinedNone
Resolvedssastry
Resolved bearND
Resolved bearND
Resolved bearND
Resolved bearND
OpenNone
Resolved Jhernandez
OpenNone
Resolved marcoil
ResolvedCatrope
Resolved marcoil
ResolvedArlolra
ResolvedArlolra
Resolved GWicke
Resolved GWicke
Resolved GWicke
Resolvedfgiunchedi
Resolvedfgiunchedi
Resolved Cmjohnson
Resolved Cmjohnson
ResolvedJoe
Resolvedfgiunchedi
Resolved GWicke
Resolved Jdouglas
Resolved GWicke
Resolved GWicke
ResolvedArlolra
Resolved GWicke
Resolved mobrovac
Resolved mobrovac
Resolved mobrovac
Resolved mobrovac
Duplicate Jdouglas
ResolvedAndrew
Resolved GWicke
Resolvedfgiunchedi
Resolvedfgiunchedi
Resolvedfgiunchedi
ResolvedEevans
Resolvedfgiunchedi
Resolved GWicke
Resolved GWicke
Resolvedfgiunchedi
Resolved mobrovac
Resolved GWicke
Resolved GWicke
Resolved AlexMonk-WMF
Resolved santhosh
Resolvedssastry
Resolved Mholloway
ResolvedJackmcbarn
ResolvedRenxiaoyi
Resolvedcscott
ResolvedKelson
OpenNone
OpenNone
OpenNone
ResolvedArlolra
ResolvedArlolra
OpenNone
DeclinedNone
OpenNone
OpenNone
DeclinedNone
DeclinedNone
DeclinedNone
OpenNone
OpenNone
InvalidNone
InvalidNone
DuplicateNone
DuplicateNone
Resolved Jhernandez
ResolvedJdlrobson
DuplicatePeter
Resolved bmansurov
DeclinedNone
DuplicateNone
Resolved nray
Resolvedphuedx
ResolvedAnomie
ResolvedAnomie
ResolvedAnomie
ResolvedEBernhardson
ResolvedAnomie
ResolvedAnomie
OpenNone
DuplicateNone
ResolvedNone
Resolvedphuedx
DeclinedNone
Resolved Pchelolo
ResolvedArlolra
Resolved Mholloway

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

I can't reproduce this on my device (Android 6, Nexus 5, version 2.1.134-alpha-2015-11-20). The link does not show in the paragraph.

Dbrant renamed this task from [2.1.134-alpha-2015-11-20 Regression] Crash on red links that are displayed as valid links to Red links displayed as regular links when Restbase enabled..Nov 24 2015, 3:44 PM
Dbrant updated the task description. (Show Details)
Dbrant set Security to None.

This is an unfortunate limitation of Parsoid, wherein it doesn't distinguish between redlinks and normal links in its output, so the app has no way of knowing which is which. T39902

Until T39902 is resolved, we could add an API call in the content service to get the list of missing pages and handle them appropriately, since the workaround is already in place in the API for Visual Editor:

https://en.wikipedia.org/w/api.php?action=visualeditor&paction=metadata&format=json&page=Staraya_Ladoga

"links": {
  "missing": [
  "Module:Location map/data/Russia Leningrad Oblast",
  "Erik Haakonsson",
  "Vodskaya pyatina",
  "Yelena River",
  "Ivan Ivanov (artist)",
  "Piotr Fomin"
  ],
},

I'd rather do this than roll the content service out to production with dead links.

Change 274638 had a related patch set uploaded (by Mholloway):
Hide redlinks

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

Change 274638 merged by Mobrovac:
Hide links to missing pages

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

I repeated the test with the 2016-03-07 release of the app. 'Yelena River' is still a clickable link, and I'm getting 'An unknown error occurred' screen instead of 'This page does not exist' screen. The link actually never loads the article but when clicking 'Read full article' that's when i get the error.

Unknown error-2016-03-07.png (854×480 px, 25 KB)

Hi @Nicholas.tsg, this is actually a server-side change that was *just* deployed (and is now being un-deployed because of a problem). Sorry, I probably moved it to the QA column prematurely. I'll update here when it's ready for testing.

This patch was reverted because it caused excessive server load in production. See T129237: Investigate server flapping after 3/7/2016 deploy.

Change 278403 had a related patch set uploaded (by Mholloway):
Hide links to missing pages, Take 2

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

Change 278403 merged by Mobrovac:
Hide links to missing pages, Take 2

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

I had to revert the new patch in production since it still caused the server to flap (as well as contained a separate issue causing a 500 error, though that's easily fixable).

On the second patch, Mobrovac wrote:

In general, the likely problem might be that all of the transforms that are done take a long time, so they prolong the event loop and come to a point of not allowing new requests to be accepted.
TL;DR this should be good enough(TM), but you will need to roll it out carefully. In the long run, we should probably think about a way to have a multi-step transform pass to allow I/O events to happen in time.

I guess the next step is to improve the efficiency of our transforms; it might be a blocker for this and is a good idea in any case.

An user notified me of this problem at a edit-a-thon today. What is the status of this?

This is still the case. I’m using the Wikipedia for Android app on Samsung Galaxy S5 mini (version 2.4.157-r-2016-09-28).

@Mholloway I don't think this warrants to be a child of the Mobile-section 2.0 task since we want to have this fixed for 1.0 as well.

Change 344546 had a related patch set uploaded (by Mholloway):
[mediawiki/services/mobileapps@master] Mark up missing links in Parsoid article HTML

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

Change 347999 had a related patch set uploaded (by Mholloway):
[mediawiki/core@master] ApiQueryLinks: add query parameter to filter by exists or !exists

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

Change 347999 abandoned by Mholloway:
ApiQueryLinks: add query parameter to filter by exists or !exists

Reason:
It seems enthusiasm for this is muted. Given that the app's issue is being solved by other means, why don't I abandon this. Maybe I'll extract and expand the new unit tests to resubmit separately another day.

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

Change 344546 abandoned by Mholloway:
Mark up missing links in Parsoid article HTML

Reason:
T39902 where are you

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

Good news! The Parsoid redlink markup feature has been deployed. I am still seeing missing links in production, but when working with a local RESTBase instance, redlinks are marked up in Parsoid HTML and removed by MCS , which tells me that in production we now just need to wait for old content to age out of cache.

Once the case in the description is verified fixed in production I'd say we can call this resolved.

...scratch that. Only deployed to beta cluster, which is also consistent with those results. Watch this space.

Change 361507 had a related patch set uploaded (by Mholloway; owner: Mholloway):
[apps/android/wikipedia@master] Update MCS content format versions for mobile-sections and definitions

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

Change 361507 merged by jenkins-bot:
[apps/android/wikipedia@master] Update MCS content format versions for mobile-sections and definitions

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