| • Niedzielski | |
| May 25 2016, 6:20 PM |
| F4667877: T136223 Galaxy Express 3 (Android 6.0.1).png | |
| Oct 27 2016, 8:26 PM |
| F4054048: screenshot-2016-05-25-12-18-43-906216102.png | |
| May 25 2016, 6:20 PM |
The Sort (C++) link preview is shown. When read article is tapped, the page is shown.
The Sort (C++) link preview spins forever and the title says "Sort (C )". When read article is tapped, a 404 is shown.
App version: 178e466e3d3a538326cdd2314a466848cfa11bc5
Android OS versions: API 23
Device model: AOSP Nexus S emulator
Device language: English
Change 290960 had a related patch set uploaded (by Dbrant):
Fix links to page titles with plus signs.
Change 290960 abandoned by Dbrant:
Fix links to page titles with plus signs.
Reason:
Scratch that; this actually appears to be a RB vs. MW issue.
Change 299872 had a related patch set uploaded (by BearND):
Don't decode hrefs from Parsoid content
Change 302745 had a related patch set uploaded (by BearND):
Change hrefs so they are the same as Mobileview API emits them
Change 299872 abandoned by BearND:
Don't decode hrefs from Parsoid content
Reason:
abandoned since I'm favoring the MCS fix I0e8e4fe011826fc4b007ed9d45b71b866dee7363
Could you summarize why the more minimal Parsoid encoding is creating issues for the app?
Change 303942 had a related patch set uploaded (by BearND):
WIP: Change way to get title from links
My MCS patch solves multiple problems:
#1 The main problem is:
To clarify this a bit more and to provide some context, the app actually takes the wiki internal links and decodes them so that it can derive the page titles from them. That's why the app adds the (intermediate) decoding step.
It's possible to work around this in the app. The main disadvantage of solving this in the app, though, is that older version of the app don't get this fix. Also other clients (e.g. the iOS app or external clients) when they transition from using MW API to MCS would have to do the same changes.
These are the alternative app patches to fix problem 1:
#2 Another problem is that the anchors in links are not anchor-encoded. So, links with anchors from the web site or other external sources don't scroll to the correct position.
#3 A third problem is that page internal links look like links to other pages because they repeat the page title before the anchor. MW API does not do that. The effect of this is that the app thinks it's loading a new page or tries to load a link preview. Ok, this is quite rare, but an example which comes to mind is that [[Cation]] is a redirect to [[Ion#Anions_and_cations]]. So, if you go to the [[Ion]] page then scroll down a bit and click on the cation link then you'll get a link preview in the app.
Here's an overview of the which link causes which issue:
| link | app master | app patch 1 | app patch 2 | app patch 3 | app master && MCS patch |
| C++ | "C " | ok | ok | ok | ok |
| 100% | ok | ok | 100%25 | ok | ok |
| #simpleAnchor (same as Ion -> cation) | preview | preview | preview | preview | ok |
| #specialCharsInAnchor | nothing(*) | nothing(*) | preview | nothing(*) | nothing (*) |
(*) The ToC entry for this has
"Special_chars_.24.25.26amp.3B" instead of "Special_chars_.24.25.26". There's a mismatch between link and TOC entry (-> separate MCS patch)
Change 304785 had a related patch set uploaded (by BearND):
Preserve title attribute in links
Change 302745 abandoned by BearND:
Change hrefs so they are the same as Mobileview API emits them
Reason:
in favor of I0e8e4fe011826fc4b007ed9d45b71b866dee7363
During the reading/services sync meeting last Thursday @Pchelolo mentioned that Parsoid actually already produces a title attribute for internal links. The MCS transforms had previously removed this attribute. The new MCS patch 2 preserves the title attributes. In combination with app patch 3 this should be good, at least for the titles.
Let's address anchor-encoding issues in a separate ticket later to keep these patches simple.
the app actually takes the wiki internal links and decodes them so that it can derive the page titles from them
It sounds like the main use case for those titles is to then construct URLs. If you kept title & URL separate, then you wouldn't need to mess with the encoding of the title portion of the URL at all.
The app displays the title of the page, too. So, it was for both display and for constructing the URL. App patch 3 separates that.
Marked it with Regression tag to better track changes other clients of MCS need to be aware of when transitioning from mobileview action to MCS.
Tested on Galaxy Express 3 (Android 6.0.1) and Wikipedia app 2.4.180-alpha-2016-10-27. According to the attached screencap, the page is shown when read article is tapped on the Sort (C++) link preview.
